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.

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

[TBD] feat(nf): Use es-module-shims during initFederation and loadRemoteModule to ensure having latest importMap #513

Conversation

jogelin
Copy link
Contributor

@jogelin jogelin commented Mar 13, 2024

Related to #489

Do not process host during initFederation

  • The Host importmap.json is directly included in index.html. No need to process it anymore.
  • The Host shared libraries are accessible before the Host is bootstrapped.
  • Host externals are directly read from es-module-shims when processing the remotes

Remote unnecessary global cache for loading the remotes

  • The processRemoteInfo and the loadRemoteModule are using directly es-module-shims
  • No DOM modifications.
  • The es-module-shims is directly used to load modules instead of caching not-updated

Process the remotes in parallel

  • Runtime initialization with a lot of remotes can be slow. Use Promises.all for processing them in parallel.

With that approach, I can now use the import map override principle natively. All of the modifications were not required but I thought it was interesting to improve the bootstrap at runtime.

I am of course open to the discussion ;)

wszydlak and others added 19 commits February 1, 2024 12:58
…al `sms-module-shims` options into `index.html`
…ms-init-options

feat(nf): Add `esmsInitOptions` to Angular builder to inject additional `esms-module-shims` options into `index.html`
…ap-to-head

fix(nf): Inject `importmap-shim` to the `<head>` instead of the end of the `<body>`
…se-script-tags

fix(nf): Remove orphans `</scripts>` tags during `updateScriptTags`
fix(nf): properly handle outputPath object
…l-peer-dep-conflict

fix(dep): fix peer dependency `browser-sync` not aligned with root dependency
…tion-config

ci: init Github actions config file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not process host during initFederation

During the build, an importmap.json is already generated.
I just adapted the code to apply the same modifications done by the deleted processHostInfo

};
}, {});

const importMap = { imports };
const exposesImports = exposes.reduce((acc, cur) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not process host during initFederation

Add the exposes when the remotes are loaded independently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remote unnecessary global cache for loading the remotes

To ensure every work like before, I covered the initFederation and the processRemoteInfo.

@@ -18,13 +16,8 @@ export async function initFederation(
? await loadManifest(remotesOrManifestUrl)
: remotesOrManifestUrl;

const hostImportMap = await processHostInfo();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not process host during initFederation

const remotesImportMap = await processRemoteInfos(remotes);

const importMap = mergeImportMaps(hostImportMap, remotesImportMap);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remote unnecessary global cache for loading the remotes

No DOM modifications.


for (const shared of remoteInfo.shared) {
const defaultExternalUrl = getDefaultExternalUrl(currentImportMap, shared);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not process host during initFederation

Host externals are directly read from es-module-shims when processing the remotes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we are using importShim<Default, Exports> from es-module-shims
I applied the same types to the loadRemoteModule

const options = normalizeOptions(optionsOrRemoteName, exposedModule);

await ensureRemoteInitialized(options);

const remoteName = getRemoteNameByOptions(options);
const remote = getRemote(remoteName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remote unnecessary global cache for loading the remotes

The es-module-shims is directly used to load modules instead of caching not-updated

}
if (fileName === '/importmap.json') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also update the generated importmap.json with the debug information like we do for the remoteEntry.json

@@ -51,5 +51,14 @@ export function updateScriptTags(
);
indexContent = indexContent.replace(/<script src="main.*?><\/script>/, '');
indexContent = indexContent.replace('</body>', `${htmlFragment}</body>`);

// add link to importmap.json
const scriptTagImportMap =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not process host during initFederation

The Host importmap.json is directly included in index.html. No need to process it anymore.

@manfredsteyer
Copy link
Contributor

Thanks for this. I'm afraid this won't work. Perhaps we have different ideas in mind.

One aspect of federation is that it negotiates the versions to use. It might decide to go with a higher minor version provided by a remote. Hence, we cannot directly include the host's import map.

Also, we cannot rely on import map overrides, as this is only supported via shims, while the idea of module federation is to eventually provide the option to work without shims.

@jogelin
Copy link
Contributor Author

jogelin commented Mar 15, 2024

One aspect of federation is that it negotiates the versions to use. It might decide to go with a higher minor version provided by a remote. Hence, we cannot directly include the host's import map.

I think I missed something sorry.
Could you indicate where in the code the negotiation of versions is done?
Is it related to the externals cache?
Because I kept the principle of using the version of the host or another remote when processing the remote (see the tests).

Also, we cannot rely on import map overrides, as this is only supported via shims, while the idea of module federation is to eventually provide the option to work without shims.

Ok, I understand.
I don't know if you saw my tweet about the single-spa agenda? It seems it is in discussion for single-spa to use es-module-shims instead of SystemJS.
Something really interesting in the post is the part:

We also discussed possible collaboration with Zack Jackson on module federation's new runtime (called MFP) and whether that could help. Also discuss whether a service worker would be necessary or helpful to hook into the browser's loading of modules.

It is an interesting idea to hook files using a service worker and it seems this is what they discussed with Zack Jackson.
I don't want to push for the overrides, the goal is not to use the overrides but to provide a way to change dynamically the URL of a module. If we use a service worker, for example on the manifest, it will not be linked to es-module-shims anymore.

In the current code, one thing to notice is that injecting the exposes in the importmap does not change anything because the exposes are always read from the cache.

That PR can be split in two.
If the injection of the host doesn't fit, the most important part is to keep the list of remotes up to date with the es-module-shims which is still used for now in the loadRemoteModule right?
My question is why do we create a cache that is not up to date when we can directly use es-module-shims as a cache?
Why do we use the importShim with an URL instead of using the key of the exposes?

@jogelin jogelin changed the title feat(nf): Use es-module-shims during initFederation and loadRemoteModule to ensure having latest importMap [TBD] feat(nf): Use es-module-shims during initFederation and loadRemoteModule to ensure having latest importMap Mar 16, 2024
@jogelin jogelin marked this pull request as draft March 16, 2024 07:18
@jogelin jogelin closed this Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants