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

Prevent infinite loop for remote circular dependencies #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rmunch
Copy link

@rmunch rmunch commented Aug 16, 2021

This fixes an infinite loop when remotes have circular dependencies. Without this, it appears that a new remote is defined and initialized even if that remote has already been created.

This is not very polished, but putting it out there in case it can help someone else.

@telenko
Copy link
Owner

telenko commented Aug 22, 2021

Hi @rmunch . Thanks for your contribution!
Still I'm not sure I'm following your changes fully. Since you have logic of prevention fetching+performing remote scripts only inside NodeModuleFederation plugin, it won't likely prevent circular dependencies inside dependencies themselves, isn't it?
I see it more reasonable to have this logic inside rpcPerform function (but then it has to be moved to a common place, currently NodeModuleFederation and NodeAsyncHttpRuntime both has own implementations). Inside rpcPerform we can have this check for an absolute script url (since with node-mf we have absolute urls only) and return cached module instance if any.
Let me know if I didn't get correctly.

@ScriptedAlchemy
Copy link

ScriptedAlchemy commented Feb 18, 2022

Okay so the problem here is your lib isn’t replicating way federation is supposed to handle remotes.

You need some global. Like window. Webpack should check if the remote was already loaded before attempting to inject the container. If the remote exists, it should call the existing get()

if a uses b which uses a, the runtime is going to get stuck in a import loop because it’s going to believe the remote doesn’t exist in memory.

This can also create context tearing since you’re not loading a single module. You’re loading a container per container.

Since the runtime chunk here is a commonjs. It's exporting a module. Not setting it globally and resolving the global as scope to interfaces

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.

3 participants