fix: clear globalLoading cache in node env #3173

2heal1 · 2024-11-05T03:55:29Z

Description

The bundler bundle GLOBAL_LOADING_REMOTE_ENTRY into share object closure ,and it can not access via globalThis .

This PR aims to solve this issue and clear globalLoading internally in revalidate method.

Related Issue

#3119

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

I have added tests to cover my changes.
All new and existing tests passed.
I have updated the documentation.

changeset-bot · 2024-11-05T03:55:33Z

🦋 Changeset detected

Latest commit: 035aa79

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 27 packages

Name	Type
@module-federation/runtime	Patch
@module-federation/node	Patch
@module-federation/devtools	Patch
@module-federation/data-prefetch	Patch
@module-federation/dts-plugin	Patch
@module-federation/nextjs-mf	Patch
@module-federation/retry-plugin	Patch
@module-federation/runtime-tools	Patch
@module-federation/webpack-bundler-runtime	Patch
@module-federation/modern-js	Patch
@module-federation/enhanced	Patch
@module-federation/rspack	Patch
@module-federation/rsbuild-plugin	Patch
@module-federation/storybook-addon	Patch
@module-federation/modernjsapp	Patch
@module-federation/sdk	Patch
@module-federation/managers	Patch
@module-federation/manifest	Patch
@module-federation/third-party-dts-extractor	Patch
@module-federation/bridge-react	Patch
@module-federation/bridge-vue3	Patch
@module-federation/bridge-shared	Patch
@module-federation/bridge-react-webpack-plugin	Patch
@module-federation/error-codes	Patch
@module-federation/esbuild	Patch
@module-federation/utilities	Patch
website-new	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

squadronai

Summary

This pull request aims to address an issue with the bundler, where the GLOBAL_LOADING_REMOTE_ENTRY was being bundled into a shared object closure, preventing it from being accessed via globalThis. The changes focus on clearing the globalLoading cache in the node environment to resolve this problem.

The key changes include:

Removing the assignment of an empty object to the __GLOBAL_LOADING_REMOTE_ENTRY__ property of the global scope in the hot-reload.ts file, which was causing issues with accessing the globalLoading cache.
Iterating over the keys in the globalLoading object and deleting them in the global.ts file, ensuring that the cache is properly cleared when the resetFederationGlobalInfo() function is called.

These modifications should help improve the overall functionality of the application by addressing the issue with the GLOBAL_LOADING_REMOTE_ENTRY and ensuring that the globalLoading cache is properly cleared in the node environment.

File Summaries

File	Summary
packages/node/src/utils/hot-reload.ts	The code changes aim to clear the `globalLoading` cache in the node environment. The changes involve removing the assignment of an empty object to the `__GLOBAL_LOADING_REMOTE_ENTRY__` property of the global scope, which was causing issues with accessing the `globalLoading` cache. This modification should help resolve the problem and improve the overall functionality of the application.
packages/runtime/src/global.ts	The code changes aim to clear the `globalLoading` cache in the node environment. This is done by iterating over the keys in the `globalLoading` object and deleting them, ensuring that the cache is properly cleared when the `resetFederationGlobalInfo()` function is called.

netlify · 2024-11-05T03:55:43Z

✅ Deploy Preview for module-federation-docs ready!

Name	Link
🔨 Latest commit	`035aa79`
🔍 Latest deploy log	https://app.netlify.com/sites/module-federation-docs/deploys/672c74cba53289000812546e
😎 Deploy Preview	https://deploy-preview-3173--module-federation-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

squadronai

Incremental Review

Comments posted: 6

Configuration

Squadron Mode: essential

Commits Reviewed

54bfd1cf730a842fd323d88d1b7c8ff72678f0ce...fb49d7518b62c3f579702986bae95a61f1ea03b1

Files Reviewed

packages/node/src/utils/hot-reload.ts
packages/runtime/src/helpers.ts

Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

.changeset/beige-cheetahs-tap.md

MadaraUchiha-314

Can I get a canary version so that I can test whether this fixes the issue ?

MadaraUchiha-314 · 2024-11-05T19:47:16Z

packages/node/src/utils/hot-reload.ts

+  Object.keys(helpers.global.globalLoading).forEach((key) => {
+    delete helpers.global.globalLoading[key];
+  });


We already have a function exposed as part of helpers:

helpers.global.resetFederationGlobalInfo();

Can we move this logic to that function ?

Ohh I forget this function , yes , we can do it , let me move it

ScriptedAlchemy · 2024-11-05T23:49:10Z

Tested and worked! @2heal1

squadronai

Incremental Review

Comments posted: 3

Configuration

Squadron Mode: essential

Commits Reviewed

fb49d7518b62c3f579702986bae95a61f1ea03b1...a62ac79219d2e78753aa26dcad8bec74894c7eb4

Files Reviewed

packages/runtime/src/global.ts

Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

.changeset/beige-cheetahs-tap.md

squadronai · 2024-11-06T03:29:58Z

packages/runtime/src/global.ts

+  Object.keys(globalLoading).forEach((key) => {
+    delete globalLoading[key];
+  });


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.

squadronai · 2024-11-06T03:29:58Z

packages/runtime/src/global.ts

  globalThis.__FEDERATION__.moduleInfo = {};
  globalThis.__FEDERATION__.__SHARE__ = {};
  globalThis.__FEDERATION__.__MANIFEST_LOADING__ = {};


Consider adding type safety checks before resetting these global objects to prevent potential runtime errors:

Suggested change

globalThis.__FEDERATION__.moduleInfo = {};

globalThis.__FEDERATION__.__SHARE__ = {};

globalThis.__FEDERATION__.__MANIFEST_LOADING__ = {};

if (!globalThis.__FEDERATION__) {

globalThis.__FEDERATION__ = {};

}

globalThis.__FEDERATION__.moduleInfo = {};

globalThis.__FEDERATION__.__SHARE__ = {};

globalThis.__FEDERATION__.__MANIFEST_LOADING__ = {};

squadronai · 2024-11-06T03:29:58Z

packages/runtime/src/global.ts

  globalThis.__FEDERATION__.moduleInfo = {};
  globalThis.__FEDERATION__.__SHARE__ = {};
  globalThis.__FEDERATION__.__MANIFEST_LOADING__ = {};
+
+  Object.keys(globalLoading).forEach((key) => {
+    delete globalLoading[key];
+  });
 }


Consider adding error handling around the global state reset operations. If any of these operations fail, it could leave the system in an inconsistent state. A try-catch block would help handle potential errors gracefully:

Suggested change

globalThis.__FEDERATION__.moduleInfo = {};

globalThis.__FEDERATION__.__SHARE__ = {};

globalThis.__FEDERATION__.__MANIFEST_LOADING__ = {};

Object.keys(globalLoading).forEach((key) => {

delete globalLoading[key];

});

}

try {

if (!globalThis.__FEDERATION__) {

globalThis.__FEDERATION__ = {};

}

globalThis.__FEDERATION__.moduleInfo = {};

globalThis.__FEDERATION__.__SHARE__ = {};

globalThis.__FEDERATION__.__MANIFEST_LOADING__ = {};

Object.assign(globalLoading, {});

} catch (error) {

console.error('Failed to clear federation cache:', error);

throw error; // Re-throw to maintain error propagation

}

2heal1 · 2024-11-06T03:54:50Z

Can I get a canary version so that I can test whether this fixes the issue ?

@MadaraUchiha-314 Yeah sure , try this 0.0.0-next-20241106033151

squadronai

Incremental Review

Comments posted: 1

Configuration

Squadron Mode: essential

Commits Reviewed

5fc6045a328c943878328da3349f945d2610c999...035aa792e5a2d3c6e0438513f77628a28263d3b6

Files Reviewed

packages/runtime/src/global.ts

Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

.changeset/beige-cheetahs-tap.md

squadronai · 2024-11-07T08:06:26Z

packages/runtime/src/global.ts

  CurrentGlobal.__FEDERATION__.moduleInfo = {};
  CurrentGlobal.__FEDERATION__.__SHARE__ = {};
  CurrentGlobal.__FEDERATION__.__MANIFEST_LOADING__ = {};


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.

squadronai bot reviewed Nov 5, 2024

View reviewed changes

module-federation deleted a comment from squadronai bot Nov 5, 2024

2heal1 changed the title ~~WIP: fix: clear globalLoading cache in node env~~ fix: clear globalLoading cache in node env Nov 5, 2024

github-actions bot added the change: fix label Nov 5, 2024

MadaraUchiha-314 approved these changes Nov 5, 2024

View reviewed changes

MadaraUchiha-314 reviewed Nov 5, 2024

View reviewed changes

ScriptedAlchemy mentioned this pull request Nov 5, 2024

Unable to fetch updated remote module code (caching issue?) #3119

Closed

5 tasks

squadronai bot reviewed Nov 6, 2024

View reviewed changes

zhoushaw approved these changes Nov 7, 2024

View reviewed changes

ScriptedAlchemy approved these changes Nov 7, 2024

View reviewed changes

2heal1 added 2 commits November 7, 2024 16:04

fix: clear globalLoading cache in node env

c3ae379

chore: move clear globalLoading logic to resetFederationGlobalInfo

035aa79

2heal1 force-pushed the feat/clear-cache-api branch from a62ac79 to 035aa79 Compare November 7, 2024 08:05

squadronai bot reviewed Nov 7, 2024

View reviewed changes

2heal1 merged commit 66ba7b1 into main Nov 7, 2024
16 checks passed

2heal1 deleted the feat/clear-cache-api branch November 7, 2024 08:17

2heal1 mentioned this pull request Nov 7, 2024

Release v0.7.1 #3192

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: clear globalLoading cache in node env #3173

fix: clear globalLoading cache in node env #3173

2heal1 commented Nov 5, 2024 •

edited

Loading

changeset-bot bot commented Nov 5, 2024 •

edited

Loading

squadronai bot left a comment •

edited

Loading

netlify bot commented Nov 5, 2024 •

edited

Loading

squadronai bot left a comment

MadaraUchiha-314 left a comment

MadaraUchiha-314 Nov 5, 2024

2heal1 Nov 6, 2024

ScriptedAlchemy commented Nov 5, 2024

squadronai bot left a comment

squadronai bot Nov 6, 2024

squadronai bot Nov 6, 2024

squadronai bot Nov 6, 2024

2heal1 commented Nov 6, 2024 •

edited

Loading

squadronai bot left a comment

squadronai bot Nov 7, 2024

fix: clear globalLoading cache in node env #3173

fix: clear globalLoading cache in node env #3173

Conversation

2heal1 commented Nov 5, 2024 • edited Loading

Description

Related Issue

Types of changes

Checklist

changeset-bot bot commented Nov 5, 2024 • edited Loading

🦋 Changeset detected

squadronai bot left a comment • edited Loading

Choose a reason for hiding this comment

Summary

netlify bot commented Nov 5, 2024 • edited Loading

✅ Deploy Preview for module-federation-docs ready!

squadronai bot left a comment

Choose a reason for hiding this comment

Incremental Review

MadaraUchiha-314 left a comment

Choose a reason for hiding this comment

MadaraUchiha-314 Nov 5, 2024

Choose a reason for hiding this comment

2heal1 Nov 6, 2024

Choose a reason for hiding this comment

ScriptedAlchemy commented Nov 5, 2024

squadronai bot left a comment

Choose a reason for hiding this comment

Incremental Review

squadronai bot Nov 6, 2024

Choose a reason for hiding this comment

squadronai bot Nov 6, 2024

Choose a reason for hiding this comment

squadronai bot Nov 6, 2024

Choose a reason for hiding this comment

2heal1 commented Nov 6, 2024 • edited Loading

squadronai bot left a comment

Choose a reason for hiding this comment

Incremental Review

squadronai bot Nov 7, 2024

Choose a reason for hiding this comment

2heal1 commented Nov 5, 2024 •

edited

Loading

changeset-bot bot commented Nov 5, 2024 •

edited

Loading

squadronai bot left a comment •

edited

Loading

netlify bot commented Nov 5, 2024 •

edited

Loading

2heal1 commented Nov 6, 2024 •

edited

Loading