Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: clear globalLoading cache in node env #3173

Merged
merged 2 commits into from
Nov 7, 2024
Merged

fix: clear globalLoading cache in node env #3173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3ae379
fix: clear globalLoading cache in node env
2heal1 Nov 5, 2024
035aa79
chore: move clear globalLoading logic to resetFederationGlobalInfo
2heal1 Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/beige-cheetahs-tap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@module-federation/runtime': patch
'@module-federation/node': patch
---

fix: clear globalLoading cache in node env
1 change: 0 additions & 1 deletion packages/node/src/utils/hot-reload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ export const performReload = async (
gs.entryChunkCache.clear();
}

gs.__GLOBAL_LOADING_REMOTE_ENTRY__ = {};
//@ts-ignore
gs.__FEDERATION__.__INSTANCES__.map((i: any) => {
//@ts-ignore
Expand Down
4 changes: 4 additions & 0 deletions packages/runtime/src/global.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ export function resetFederationGlobalInfo(): void {
CurrentGlobal.__FEDERATION__.moduleInfo = {};
CurrentGlobal.__FEDERATION__.__SHARE__ = {};
CurrentGlobal.__FEDERATION__.__MANIFEST_LOADING__ = {};
Comment on lines 113 to 115
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code resets global federation state without checking if these properties exist first. Add property existence checks to prevent potential undefined errors:

Suggested change
CurrentGlobal.__FEDERATION__.moduleInfo = {};
CurrentGlobal.__FEDERATION__.__SHARE__ = {};
CurrentGlobal.__FEDERATION__.__MANIFEST_LOADING__ = {};
if (CurrentGlobal.__FEDERATION__.moduleInfo) CurrentGlobal.__FEDERATION__.moduleInfo = {};
if (CurrentGlobal.__FEDERATION__.__SHARE__) CurrentGlobal.__FEDERATION__.__SHARE__ = {};
if (CurrentGlobal.__FEDERATION__.__MANIFEST_LOADING__) CurrentGlobal.__FEDERATION__.__MANIFEST_LOADING__ = {};

This defensive programming approach prevents errors if any of these objects haven't been initialized yet.

Note: The existing comments already cover other important aspects like type safety, error handling, and the globalLoading optimization, so I've focused on this additional safety concern.


Object.keys(globalLoading).forEach((key) => {
delete globalLoading[key];
});
Comment on lines +117 to +119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current approach to clearing the globalLoading object could be simplified using Object.keys().length. A more efficient way would be to reassign the object:

Suggested change
Object.keys(globalLoading).forEach((key) => {
delete globalLoading[key];
});
// Reset globalLoading to empty object
Object.assign(globalLoading, {});

This achieves the same result with better performance, especially when dealing with objects that have many properties.

}

export function getGlobalFederationInstance(
Expand Down
Loading