telenko / node-mf

57 stars 6 forks source link

Prevent infinite loop for remote circular dependencies #6

Open rmunch opened 3 years ago

rmunch commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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