@gioboa

@gioboa

Yep, I was looking at this.

It looks great to me 👏👏 Can we recreate the import scenario in the multi example e2e test please?

I can try though that may take a bit, will be lower priority for me.

@gioboa can you explain how exactly the multi-example pnpm script works/what it is launching. trying to dig into that is confusing.

And given this effectively requires a config change, does that mean needing to copy and duplicate an app to simulate any of this?

Do you have suggestions on the best way to test this given its an internal detail and not a visual one?

Thanks.

can you explain how exactly the multi-example pnpm script works/what it is launching. trying to dig into that is confusing.

We have the e2e script that is using playwright here.

Do you have suggestions on the best way to test this given its an internal detail and not a visual one?

We can modify the config on one remote to use the new import:false setting. If the code is working properly the test will pass. WDYT?

can you explain how exactly the multi-example pnpm script works/what it is launching. trying to dig into that is confusing.

We have the e2e script that is using playwright here.

Do you have suggestions on the best way to test this given its an internal detail and not a visual one?

We can modify the config on one remote to use the new import:false setting. If the code is working properly the test will pass. WDYT?

That still leaves me confused as to what is running where.

We can modify the config on one remote to use the new import:false setting That would depend of if the remote is using a shared import or not b/c changing a remote that doesn't share anything is a no-op?

That still leaves me confused as to what is running where.

the npm script is this one "e2e": "playwright test"

That would depend of if the remote is using a shared import or not b/c changing a remote that doesn't share anything is a no-op?

I did a test with your branch.
I added and used lodash in the dynamic remote, I changed the vite config

shared: {
  react: { singleton: true },
  'react-dom': { singleton: true },
  lodash: { import: false },
},

import _ from 'lodash';

const SignUpBanner = () => {
  console.log(`Shared lodash v${_.VERSION}`);
  [...]
};

export default SignUpBanner;

and I added lodash in the host dependencies too.

I run pnpm multi-example and when I open and close the dynamic component, the new import logic is executed

but the function loadShare in the module -federation/runtime-core is not returning anything 🤔

That still leaves me confused as to what is running where.

the npm script is this one "e2e": "playwright test"

That would depend of if the remote is using a shared import or not b/c changing a remote that doesn't share anything is a no-op?

I did a test with your branch. I added and used lodash in the dynamic remote, I changed the vite config

shared: {
  react: { singleton: true },
  'react-dom': { singleton: true },
  lodash: { import: false },
},

import _ from 'lodash';

const SignUpBanner = () => {
  console.log(`Shared lodash v${_.VERSION}`);
  [...]
};

export default SignUpBanner;

and I added lodash in the host dependencies too.

I run pnpm multi-example and when I open and close the dynamic component, the new import logic is executed

but the function loadShare in the module -federation/runtime-core is not returning anything 🤔

Your missing what im trying to ask. What is being tested for the host and remote in examples/. Im not asking about the playright code but the vite servers themselves on multi-example. That is what im confused b/c I don't see a multi-example package so im lost on what vite apps are being launched and for what role.

The e2e test is simple but useful test to verify the library, we are testing if the milti example app is still working or not.

The e2e test is simple but useful test to verify the library, we are testing if the milti example app is still working or not.

Sorry your still not getting it.

Im asking what apps in the examples folder are being started, and what their roles are respectfully for the tests.

It's examples/vite-webpack-rspack
This is the app

shared: {
react: { singleton: true },
'react-dom': { singleton: true },
lodash: { import: false },
},

I got it working though will need a better way to verify things since the promo/banner seem to be random.

The one key is that you have to put in a static reference in the host App.tsx for it to correctly put it in the shared items. You cannot bundle a dependency and the host not use it.

So

import _ from 'lodash';
_.VERSION

is required.

console.log(`Shared lodash v${_.VERSION}`);`

will trigger in the bundle a call to loadShared which will have the sharemap already passed down from the parent though init proces in remoteEntry.js.

Is there any specific real test you would like for this?

@gioboa this is ready now.

I also in the processed have reproduced #132

If you have a shared dependency but its not included in package json, it breaks ( Cannot read properties of null (reading 'id') [plugin vite:dep-pre-bundle]). You can likely reprod by deleting lodash from dynamic-remote package.json and try to run it. I did a lot of troubleshooting before I noticed the build for it failing.

How do you think that should be handled since we know the likely cause. Would be a separate PR.

How do you think that should be handled since we know the likely cause. Would be a separate PR.

with that check the PR is self contained, so let's add it please. Thanks for your commitment 💪

Sorry, what. your being cryptic 🤣

I'm sorry, let's add the check to prevent this error Cannot read properties of null (reading 'id') [plugin vite:dep-pre-bundle]

That should be in its own separate PR b/c its not actually connected, I just found the error by chance, so I want to know what you want the behavior to be. Silently ignore, throw an error? console.error?

I can submit a PR for that once you clarify and thats a very simple change.

I'm sorry, let's add the check to prevent this error Cannot read properties of null (reading 'id') [plugin vite:dep-pre-bundle]

That should be in its own separate PR b/c its not actually connected, I just found the error by chance, so I want to know what you want the behavior to be. Silently ignore, throw an error? console.error?

I can submit a PR for that once you clarify and thats a very simple change.

Options:

• Silently ignore: is not good because the developer can improve the code and remove the shared dependency.
• throw an error: will stop you to build and deploy
• console.error: will fill your browser console and most of the time this kind of errors are ignored by devs

So based on that, throw an error should be fine. What do you think?

I'm sorry, let's add the check to prevent this error Cannot read properties of null (reading 'id') [plugin vite:dep-pre-bundle]

That should be in its own separate PR b/c its not actually connected, I just found the error by chance, so I want to know what you want the behavior to be. Silently ignore, throw an error? console.error?
I can submit a PR for that once you clarify and thats a very simple change.

Options:

• Silently ignore: is not good because the developer can improve the code and remove the shared dependency.
• throw an error: will stop you to build and deploy
• console.error: will fill your browser console and most of the time this kind of errors are ignored by devs

So based on that, throw an error should be fine. What do you think?

I can throw an error. as for console.error i meant in vite, not the browser lol.

Anyways ill make that PR quickly. Please review and merge this.

I run the multi-example with your code on my local env. I added a console log here to verify the new code

but I can't see that log. 🤔

I dug into it.

https://github.com/module-federation/core/blob/f045a458fcadc4b5859cafd6c198d07749b0cb04/packages/runtime-core/src/shared/index.ts#L92

Because the remotes share map does no .lib prop, it effectively gets skipped. So the code can be seen as dead code, but I structured it based on what ScriptedAlchemy asked for. So... because it rewrite the actual resolve (alias config) to a stub that calls useShared, we already didn't have it using its own version, it just bundled in a duplicate anyways.

So this kind of makes an opt-in option for something that just removes dead code and sends a flag to the MF runtime...

@gioboa Is there any further you need me to do on this PR? Thanks.

As mentioned, the new code is currently unused, so I think we should find a different way to support the import false configuration.

That simply then means just not rendering the dead logic, but that goes against what scriptedachamy advised in the previous PR, though it also means the import: false is never sent to MF anyways and it becomes closer to my original approach.

Thoughts?

Thoughts?

Can we have your opinion @ScriptedAlchemy on this please?

I run the multi-example with your code on my local env.
I added a console log here to verify the new code

but I can't see that log. 🤔

Can't we just not render that template string part

I run the multi-example with your code on my local env.
I added a console log here to verify the new code

but I can't see that log. 🤔

Can't we just not render that template string part

You were kind of against that saying it breaks the protocol 🙃 . I used what you wanted me to do 😝

no, you deleted the entire template object. Im saying have a get method return a throw else return the original. C'mon

export function generateLocalSharedImportMap() {
  const options = getNormalizeModuleFederationOptions();
  return `
    import {loadShare} from "@module-federation/runtime";
    const importMap = {
      ${Array.from(getUsedShares())
        .sort()
        .map((pkg) => {
          const shareItem = getNormalizeShareItem(pkg);
          return `
        ${JSON.stringify(pkg)}: async () => {
          ${
            shareItem?.shareConfig.import === false
              ? `throw new Error(\`Shared module '\${${JSON.stringify(pkg)}}' must be provided by host\`);`
              : `let pkg = await import("${getPreBuildLibImportId(pkg)}");
            return pkg;`
          }
        }
      `;
        })
        .join(',')}
    }
      const usedShared = {
      ${Array.from(getUsedShares())
        .sort()
        .map((key) => {
          const shareItem = getNormalizeShareItem(key);
          if (!shareItem) return null;

          const getBody =
            shareItem.shareConfig.import === false
              ? `
              const shared = await loadShare(${JSON.stringify(key)});
              if (shared) return () => shared;
              throw new Error(\`Shared module '\${${JSON.stringify(key)}}' must be provided by host\`);
              `
              : `
              usedShared[${JSON.stringify(key)}].loaded = true
              const {${JSON.stringify(key)}: pkgDynamicImport} = importMap
              const res = await pkgDynamicImport()
              const exportModule = {...res}
              // All npm packages pre-built by vite will be converted to esm
              Object.defineProperty(exportModule, "__esModule", {
                value: true,
                enumerable: false
              })
              return function () {
                return exportModule
              }
              `;

          return `
          ${JSON.stringify(key)}: {
            name: ${JSON.stringify(key)},
            version: ${JSON.stringify(shareItem.version)},
            scope: [${JSON.stringify(shareItem.scope)}],
            loaded: false,
            from: ${JSON.stringify(options.name)},
            async get () {${getBody}
            },
            shareConfig: {
              singleton: ${shareItem.shareConfig.singleton},
              requiredVersion: ${JSON.stringify(shareItem.shareConfig.requiredVersion)},
              ${shareItem.shareConfig.import === false ? 'import: false,' : ''}
            }
          }
        `;
        })
        .filter((x) => x !== null)
        .join(',')}
    }
      const usedRemotes = [${Object.keys(getUsedRemotesMap())
        .map((key) => {
          const remote = options.remotes[key];
          if (!remote) return null;
          return `
                {
                  entryGlobalName: ${JSON.stringify(remote.entryGlobalName)},
                  name: ${JSON.stringify(remote.name)},
                  type: ${JSON.stringify(remote.type)},
                  entry: ${JSON.stringify(remote.entry)},
                  shareScope: ${JSON.stringify(remote.shareScope) ?? 'default'},
                }
          `;
        })
        .filter((x) => x !== null)
        .join(',')}
      ]
      export {
        usedShared,
        usedRemotes
      }
      `;
}

Other than that - this is pretty much exactly what I was expecting. Well done.

@gioboa @ScriptedAlchemy done.

@gioboa following up.

@gioboa following up.

I'm waiting the feedback from ScriptedAlchemy

@gioboa following up.

I'm waiting the feedback from ScriptedAlchemy

@ScriptedAlchemy ping 🙃

@ScriptedAlchemy can you please review.