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

Improve version handling in native federation #399

Open
JBBianchi opened this issue Dec 6, 2023 · 2 comments · May be fixed by #400
Open

Improve version handling in native federation #399

JBBianchi opened this issue Dec 6, 2023 · 2 comments · May be fixed by #400

Comments

@JBBianchi
Copy link

Hello,

Afaik (which might be wrong), there is not built-in way to handle the requiredVersion of SharedInfo when processing a remote.

There seems to be an intention with externals but I fail to see how they are supposed to enforce version compatibility.

My take would be to look at the current importMap.imports for the target package version and apply semver ruling to conditionally import it at the scope level or not.

Wdyt ?

JBBianchi added a commit to JBBianchi/module-federation-plugin that referenced this issue Dec 6, 2023
@JBBianchi JBBianchi linked a pull request Dec 6, 2023 that will close this issue
@okamiconcept
Copy link

I've just seen this very odd hole in the code too...

So basically there is some api configuration copied from module federation (like requiredVersion or strictVersion) but they are not used at all in the native federation implementation (yet)

I am a bit preoccupied that this bug has not been fixed yet (December 23) since its a major feature missing from the federation.
To be more clear we are forced to have exactly the same angular version (or other lib) in the host and the remote to have the same module reconciliation. It is more accurate for the Angular version since we do not have the same injector context when we don't have the same Angular version.

Maybe we can just change the way the "externals" are stored to keep in mind the required version and / or the strict version, like @JBBianchi proposed with semver.

Please help us having a fully featured package! :)

And congrats for you efforts to bring this native federation implementation, it is such a good idea!

BTW: What was your solution to bypass this @JBBianchi?

@okamiconcept
Copy link

I've created a patch with the patch-package npm script in the meantime:

diff --git a/node_modules/@softarc/native-federation-runtime/fesm2022/softarc-native-federation-runtime.mjs b/node_modules/@softarc/native-federation-runtime/fesm2022/softarc-native-federation-runtime.mjs
index 3785c8a..d8b1093 100644
--- a/node_modules/@softarc/native-federation-runtime/fesm2022/softarc-native-federation-runtime.mjs
+++ b/node_modules/@softarc/native-federation-runtime/fesm2022/softarc-native-federation-runtime.mjs
@@ -1,3 +1,5 @@
+import { satisfies } from 'semver';
+
 function mergeImportMaps(map1, map2) {
     return {
         imports: { ...map1.imports, ...map2.imports },
@@ -13,12 +15,26 @@ global[nfNamespace] ??= {
     baseUrlToRemoteNames: new Map(),
 };
 const globalCache = global[nfNamespace];
+const packageKeySeparator = '|';
 
 const externals = globalCache.externals;
 function getExternalKey(shared) {
-    return `${shared.packageName}@${shared.version}`;
+    return `${shared.packageName}${packageKeySeparator}${shared.version}`;
 }
 function getExternalUrl(shared) {
+    if (!shared.strictVersion) {
+      // retrieve all packages with the same package name and the resoluted required version
+      const packages = Array.from(externals.keys()).filter((k) => {
+        const p = k.split(packageKeySeparator);
+        const packageName = p[0];
+        const version = p[1];
+        return packageName === shared.packageName && satisfies(version, shared.requiredVersion);
+      });
+      if (packages.length) {
+        return externals.get(packages[0]);
+      }
+    }
+
     const packageKey = getExternalKey(shared);
     return externals.get(packageKey);
 }

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 a pull request may close this issue.

2 participants