-
Notifications
You must be signed in to change notification settings - Fork 71
feat: add remotePlugin option to control shared module bundling #327
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
base: main
Are you sure you want to change the base?
Conversation
- Introduces new `remotePlugin` option to prevent shared modules from being bundled into remotes - When enabled, remotes will rely on host-provided shared modules at runtime - Updates related virtual modules and plugin logic to respect the new option - Conditionally skips hostInit entry addition when remotePlugin is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR @pcfreak30
I would like to hear the feedback from @ScriptedAlchemy
Please let us know what you think
Should be able to accomplish the same thing with import:false on a share package. Remotes shouldn't be bundling into the remote anyway. So there should be no size increase to require this. If you want to control module loading and share resolution. Federation runtime plugins are the appropriate way to do this. In short, the capability should already exist and not require another plugin. If theres double bundling. Thats a bug in the existing plugin |
As i stands what I have is working perfectly fine. The bundling is because of static references. if you have them in the imports map they will get included even if the share map uses the host. This is a build time issue not a runtime issue.... Also understand I do not pass remotes in at build time. my use case isn't any of the existing examples and I do not have multiple So with the flag I added, i shortcut the imports and prevent them from being bundled while having the prebuild rewrite code reroute them to loadShare. So yea, this is different then what you are describing I think. and |
Okay. So the specification of federation. To treat shared modules as externals who are not bundled in. One would set import: false as a option on the shared object for that library. Which essentially means, dont bundle it and rely only on the parent application to supply it, the get factory would just return throw module not found if remote attempts to use its own getter and not the hosts. The correct way to implement this would be a false capability on the import option which would then do the reroute to loadShare. Is this a compiler speed issue? Why not bake in resilience with a few extra chunks? If theres a network issue - the system would have the ability to recover from failure. Generally I try to encourage users from introducing single point of failures into a distributed system. But since you want fancy requirejs, import: false would be the way to go over adding another plugin, rather update share plugin and options to align with our specifications. Another important consideration is our runtime is generally designed to expect our specifications. For instance, we may read compiler options in other parts of the ecosystem. For instance deployment layers rely on the spec. Compile time, import false replaces the getter to the fallback module with a throw error, and uses loadShare to fetch it elsewhere. Runtime doesn't support import false afik since we would just pass a throw as the get() or lib() on the init object - typically thats what the compiler does for us. |
If i mark shared as external, then it wont process at all for the Its nothing to do with compiler speed, not sure where you got that from? I never mentioned anything about speed. This is a build time problem, and the vite plugin doesn't do anything with the federation runtime library during build other then bundle it? |
We dont mark it as external. Its simply not added as a dependency and code generation renders a throw on the getter instead and its rerouted to loadShare. Don't make new plugin. Do same logic, just if shared.react.import = false Approach is right. Implementation is wrong 🥰
Im still failling to see the point in why you dont want the artifacts to simply exist still? You can say 'only use host shared resources' and still keep the others as fallbacks. That aside, import: false is the correct way to do it |
Module Federation remotePlugin PR Analysis - Complete Technical ReviewExecutive SummaryAfter examining the LumeWeb/module-federation-vite remote-plugin branch source code and the Module Federation runtime-core implementation, the PR Implementation AnalysisWhat the PR Actually DoesThe
Author's Use CaseThe PR addresses a legitimate architectural pattern:
Runtime Failure AnalysisCritical Issue: Misunderstanding of Shared Module SystemAfter examining the actual virtualRemoteEntry.ts implementation and runtime-core source, the What remotePlugin: true Actually Does// From virtualRemoteEntry.ts - generates EMPTY configuration
const importMap = {
${options.remotePlugin ? '' : Array.from(getUsedShares()).map(pkg => generateShareImport(pkg)).join(',')}
}
const usedShared = {
${options.remotePlugin ? '' : Array.from(getUsedShares()).map(key => generateShareConfig(key)).join(',')}
} This creates a remote with no shared module declarations whatsoever. The Fundamental MisunderstandingIncorrect assumption: "If remote doesn't declare shared dependencies, it will automatically use host's versions" Reality: Module Federation requires explicit coordination between containers. A remote that doesn't declare its shared dependencies cannot participate in the sharing protocol. Actual Runtime Flow AnalysisWhen a consumer (remote or host) tries to access a shared module: import React from 'react'; // Generated code calls loadShare('react') Step 1: loadShare() Called on Consumer // /packages/runtime-core/src/shared/index.ts:111-117
async loadShare<T>(pkgName: string): Promise<false | (() => T | undefined)> {
const shareInfo = getTargetSharedOptions({
pkgName,
shareInfos: host.options.shared, // Consumer's own shared config
});
// ...
} Step 2: getTargetSharedOptions() Looks in Consumer's Config // /packages/runtime-core/src/utils/share.ts:289-318
export function getTargetSharedOptions(options: {
shareInfos: ShareInfos; // Consumer's own shared config
}) {
const defaultResolver = (sharedOptions: ShareInfos[string]) => {
if (!sharedOptions) {
return undefined; // ← No config found in consumer
}
};
return resolver(shareInfos[pkgName]); // undefined if not declared
} Step 3: Assertion Failure // /packages/runtime-core/src/shared/index.ts:152-155
assert(
shareInfoRes, // ← undefined when consumer has no config
`Cannot find ${pkgName} Share in the ${host.options.name}.`
); Key insight: The Correct Sharing Flow// How sharing SHOULD work:
// 1. Remote declares what it needs:
const usedShared = {
react: {
version: "18.0.0",
get: async () => import("react"), // Fallback
shareConfig: { singleton: true, import: false } // Don't bundle, but declare requirement
}
}
// 2. At runtime, loadShare('react') finds this config
// 3. Runtime can then coordinate with host to get shared version
// 4. If host doesn't provide, falls back to remote's get() function Why This Approach Cannot WorkThe
Expected vs Actual BehaviorWhat the author expects: // Remote with remotePlugin: true
import React from 'react'; // "Should automatically use host's React" What actually happens: // Generated remote entry code
const usedShared = {}; // Empty - no React configuration
// At runtime when import is processed:
loadShare('react')
→ getTargetSharedOptions({ shareInfos: {} }) // Empty config
→ returns undefined
→ assert(undefined) // Throws error
→ Application crashes What should happen (correct approach): // Remote declares requirement but doesn't bundle
const usedShared = {
react: {
shareConfig: { import: false, singleton: true },
get: () => { throw new Error("Host must provide React") }
}
}
// At runtime:
loadShare('react')
→ getTargetSharedOptions finds react config
→ Coordinates with host through share scope
→ Returns host's React or throws configured error Architectural AnalysisVite Plugin vs Webpack Plugin DifferenceWebpack Module Federation: // import: false still maintains registration
shared: {
react: {
import: false, // Don't bundle locally
singleton: true, // Still registered in share scope
requiredVersion: "^18.0.0"
}
}
// Runtime knows about the module and can negotiate with host Vite Plugin with remotePlugin: true: // Completely empty - no registration at all
const usedShared = {}
// Runtime has no knowledge of any shared modules The Fundamental ProblemThe
Comparison: remotePlugin vs import: falseremotePlugin: true Approach
import: false Approach
Key Runtime Difference// import: false behavior
loadShare('react') →
getTargetSharedOptions(): finds react config with import: false →
Coordinates with share scope to find host's version →
Returns host's React or fallback
// remotePlugin: true behavior
loadShare('react') →
getTargetSharedOptions(): shareInfos is {} (empty) →
defaultResolver(undefined): returns undefined →
assert(undefined): THROWS ERROR → application crashes Working Alternatives1. Use import: false (Recommended)// In remote configuration
shared: {
react: {
import: false, // Don't bundle
singleton: true, // Use host's version
requiredVersion: '^18.0.0'
}
} Benefits:
2. Maintainer-Recommended Approach: Enhanced import: falseBased on ScriptedAlchemy's feedback in the PR discussion, the correct approach is: // In remote configuration - modify existing share plugin behavior
shared: {
react: {
import: false, // Don't bundle locally
singleton: true, // Use host's version
requiredVersion: '^18.0.0',
// Proposed enhancement: throw error on fallback getter
// Runtime uses loadShare() to fetch from host
}
} Maintainer's suggested implementation:
3. Don't Use Module FederationIf complete isolation is required:
Ecosystem and Long-term Sustainability ConcernsBeyond the immediate runtime failures, the 1. Behavioral Deviation from Specification
2. Guaranteed Rolldown Migration Failure
3. Guaranteed Runtime Incompatibility
4. Maintenance Burden
Final VerdictThe remotePlugin: true implementation should be rejected due to multiple critical issues: Technical Problems:
Ecosystem Problems:
Evidence from Source Code:
Recommendation:The approach addresses a legitimate use case but is fundamentally based on a misunderstanding of Module Federation's sharing protocol. Root cause: The author assumes "no shared config = use host's modules" but the reality is "no shared config = cannot participate in sharing at all" Correct solutions:
The |
Claude more or less confirms my concern with the implementation. |
js bloat. i dont want dead code in the bundle at all. There is no reason to have it in bundle. So again, short cutting and skipping the entire import map solves that.
Incorrect for me. The hosts shared deps map get copied to the remote. im manually calling registerRemote and loadRemote in a framework which is after the host has init'ed its own FederationHost. Skipping hostInit might not be needed but I would have to re-test. And even if im using MF in a unique way, ive already built the system, and there is no point in trying to rebuild it. I understand the core fairly deeply now as I have spent a lot of time in a debugger with it. To be absolutely clear, this change is working for my needs in a development environment. Your assumptions are based on auto-init of everything from a bundler build for it to auto-wire everything, I think, where as I am taking more low level control of the remotes and managing MF myself. |
May give you some more context. |
hmm, how would it broker shared versions and semver? Based on my understanding, the remove passes absolutely no information at all, not even what it needs for sharing, no version info, singleton flags or requirements? Are you using loadShare by hand as well? or is that still delegated by the plugin? If you had 2 shares at different versions the remote seems to not have any share information at all, maybe just a vanilla loadRemote('react') and randomly picks one? regarding js bloat, so we are on the same page - you mean:
Thats the bloat? Where does your host init itself in your source? I do want to point out that federation is technically in a broken state in your implementation, if there were ever another module you attach to the system who also shares modules - the application will break. If you would like the proposed feature, update it to use import: false, then filter the shareMap so that the get() is just a dead function / noop instead of a dynamic import, but the version information and singleton flags are still preserved, basically keep everything but the import() that you dont want. Alternatively, if you are satisfied with what you have, forking may be a better approach to ensure this remains working after the rolldown switch takes place. Again, your use case is valid - but supposed to be implemented with import: false like we have in webpack,rspack,metro,rolldown. These pugins, will they remain under your control only? user will never be able to add their own right? |
So, right now im the only author of plugins. I expect that to change in the long future, but the system is very early right now in age. frontend plugins are embedded into a golang plugin (go embed), and served. It works based on how caddy operates in design. sem ver changes aren't really a concern right now and so I haven't focused or tested on that...
no, the import map.
I am keeping the shared map IIRC. The core ISSUE is that by having
The focus is mostly on the remotes behavior. That isn't updated for this patch, but I also didn't change how the host build behaves...
okay yeah, so if you want to get rid og the import(), then import: false pattern on the share and replace the import() with ()=> {console.error('shared fallback module not available')} for cases where imoprpt is false, else import(thedep) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ScriptedAlchemy for sharing your thoughts and experience here.
this remains working after the rolldown switch takes place.
Do you have an ETA for this?
we are writing the specs for them now. would have to ask evan |
😅 Yep, of course. Your insight is interesting too, so something is moving in the right direction 👌 Thanks for the update 🤩 |
yes it is. :D |
@ScriptedAlchemy thoughts on my last response? |
@gioboa what is your opinion on this PR based on the discussion? |
@ScriptedAlchemy shared the mfe team experience on this topic, on the other hand, if your solution is working fine for you, go for it. |
And he has yet to respond to my last responses, because I still don't think it has been understood clearly that this is a build time problem, not a runtime problem. |
After conducting a comprehensive analysis of both the Vite federation codebase and this PR, I need to respectfully disagree with the current implementation. While your use case is absolutely legitimate, the The Core ProblemYour current implementation creates an empty share map (
What You're Trying to Achieve vs. What's HappeningYour Goal (Valid): // You want: No bundling of shared modules in remotes
// Host provides ALL shared dependencies
// Plugin-style architecture with manual control Current Implementation (Problematic): // remotePlugin: true creates:
const usedShared = {}; // Empty - breaks MF protocol What Should Happen Instead: const usedShared = {
"@lumeweb/portal-framework-core": {
name: "@lumeweb/portal-framework-core",
version: "0.0.0",
scope: ["default"],
loaded: false,
from: "host",
async get() {
// Throw error when not available from host, since theres no local fallback
throw new Error("Cannot get '@lumeweb/portal-framework-core' - must be provided by host");
},
shareConfig: {
singleton: true,
requiredVersion: "^0.0.0",
import: false // This tells MF theres no local fallback
}
}
}; The Root Issue: Vite Federation Lacks
|
@ScriptedAlchemy your focusing too much on the usedShared part vs imports map. the shared aspect is less of a concern. The imports map is what caused tree shaking to be impossible and causes bundling. Sure the imports support in If removing the imports breaks the Oh and im not moving to rspack b/c realistically my whole system uses vite and thats a no-go eng wise. I have things working fine atm, so its just a matter of my changes being tweaked to fit both the standard and my requirements... Thoughts? |
I feel the solution i provided you at least 4 times now does what you want. Which is removes the import() from the object and replaces it with a throw etc. Btw: shared modules should not tree shake in general. Since they are used in unknown ways. But thats a separate discussion from your request. Import:false, when false dont add a dybamic import in the code generation part. Keep everything else, just not the import() when a share has import false set |
@ScriptedAlchemy I can do that but.. the import property isn't part of the spec atm? https://github.com/module-federation/core/blob/0f1e3a9d9f3cc1012f263bb30ee6825df0a512dd/packages/runtime-core/src/type/config.ts#L49 |
If not then we dont need to add it to the code generation. Just built time option then |
can you please clarify? |
// In virtualRemoteEntry.ts - handle import: false case // vite.config.js |
the runtime types themselves dont support / care about |
remotePlugin
option to prevent shared modules from being bundled into remotesI am opening this up as a draft PR for discussion on what this should be named, or how it should be allowed to be configured.
Fundamentally my use case is I use MF as a glorified
requirejs
module loader without iframes or new web pages.I have a plugin system where all shared deps are on the host app, as if it was an electron app shell. Thus I have to prevent double bundling of those dependencies... It is also a huge performance bloat to load 5-6 large libraries in every single plugin (remote bundle).
This is part of a series of PR's to upstream my R&D.