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

feat: support node esmodule and mf-manifest esmodule #2934

Merged

Conversation

zhangHongEn
Copy link
Contributor

@zhangHongEn zhangHongEn commented Sep 8, 2024

Description

Supports importing remote files in esm format in the node environment

Related Issue

module-federation/vite#73

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation.

Copy link

changeset-bot bot commented Sep 8, 2024

⚠️ No Changeset found

Latest commit: 24317ec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented Sep 8, 2024

Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 24317ec
🔍 Latest deploy log https://app.netlify.com/sites/module-federation-docs/deploys/66f87b7537f5b600086319b6
😎 Deploy Preview https://deploy-preview-2934--module-federation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This pull request adds support for importing remote files in ESM (ECMAScript Module) format in the Node.js environment. The key changes are as follows:

  • Update the moduleInfo object to handle the type property, which can now be set to 'esm' or 'global' depending on the remote module's format.
  • Introduce a new type property in the remoteInfo object, which is then passed to the loadScriptNode function to handle the different script types (CommonJS or ESM) and ensure proper loading of the remote entry.
  • Update the script type handling in the dom.ts file, adding the ability to set custom attributes on the created script element.
  • Modify the generateSnapshotFromManifest function to set the ssrRemoteEntryType property based on the type field of the ssrRemoteEntry object, allowing for both CommonJS and ESM formats.
  • Add a new loadModule function that handles the loading and evaluation of ESM modules, and update the createScriptNode function to support ESM scripts by leveraging the loadModule function.

These changes aim to improve the integration of the module federation library with the Node.js environment, allowing for the seamless handling of remote files in both CommonJS and ESM formats.

File Summaries
File Summary
packages/runtime/src/plugins/generate-preload-assets.ts The code changes introduce support for importing remote files in ESM format in the Node environment. This includes updating the moduleInfo object to handle the type property, which can now be set to 'esm' or 'global' depending on the remote module's format.
packages/runtime/src/utils/load.ts The code changes introduce support for importing remote files in ESM format in the Node.js environment. The key modifications include adding a new type property to the remoteInfo object, which is then passed to the loadScriptNode function. This allows the function to handle the different script types (CommonJS or ESM) and ensure proper loading of the remote entry.
packages/sdk/src/dom.ts The code changes introduce support for importing remote files in ESM format in the Node environment. The primary modifications include updating the script type based on the provided attributes, and adding the ability to set custom attributes on the created script element.
packages/sdk/src/generateSnapshotFromManifest.ts The code changes introduce support for importing remote files in ESM format in the Node.js environment. The primary modification is to the generateSnapshotFromManifest function, where the ssrRemoteEntryType property is now set based on the type field of the ssrRemoteEntry object, allowing for both CommonJS and ESM formats.
packages/sdk/src/node.ts The code changes introduce support for importing remote files in ESM format in the Node.js environment. The primary modifications include adding a new function loadModule that handles the loading and evaluation of ESM modules, and updating the createScriptNode function to support ESM scripts by leveraging the loadModule function.

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 5

Configuration

Squadron Mode: essential

Commits Reviewed

760cc35c53dd9beb7ebba20b729108b2c61b0966...d9d4a22d7b269162675622e19bd675bd1357a3f8

Files Reviewed
  • packages/runtime/src/utils/load.ts
  • packages/sdk/src/generateSnapshotFromManifest.ts
  • packages/sdk/src/node.ts

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 0

Configuration

Squadron Mode: essential

Commits Reviewed

d9d4a22d7b269162675622e19bd675bd1357a3f8...e60dabaca046ef623c926a245cd9377aa5fccb4f

Files Reviewed
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 0

Configuration

Squadron Mode: essential

Commits Reviewed

e60dabaca046ef623c926a245cd9377aa5fccb4f...9cb92db6d1e2e16baa870402dccc7c05a1976379

Files Reviewed
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json
  • packages/bridge/vue3-bridge/vite.config.ts.timestamp-1725810701228-7f307fef673c.mjs

@ScriptedAlchemy
Copy link
Member

please run nx:format write

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 0

Configuration

Squadron Mode: essential

Commits Reviewed

88214aa616d11842bdc16af3b4805310b064ffb9...d58b32713d1e038764fbdf00681741b4ed8813e4

Files Reviewed
  • packages/sdk/src/generateSnapshotFromManifest.ts
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

@zhangHongEn
Copy link
Contributor Author

please run nx:format write

Thanks, it's now complete. I've run the fix on my home computer before, but it didn't take effect.

@module-federation module-federation deleted a comment from squadronai bot Sep 10, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 10, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 10, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 10, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 10, 2024
Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 3

Configuration

Squadron Mode: essential

Commits Reviewed

59758b7e0d6244a781cddf7e895743668164479c...14407ec24f82f99e20fe436c239d6de161b1e86b

Files Reviewed
  • packages/runtime/src/utils/load.ts
  • packages/sdk/src/generateSnapshotFromManifest.ts
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 0

Configuration

Squadron Mode: essential

Commits Reviewed

59758b7e0d6244a781cddf7e895743668164479c...ef194a1236fb6717c2f1ba9e9516f102097445fb

Files Reviewed
  • packages/runtime/src/utils/load.ts
  • packages/sdk/src/generateSnapshotFromManifest.ts
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

squadronai[bot]

This comment was marked as resolved.

@module-federation module-federation deleted a comment from squadronai bot Sep 25, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 25, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 25, 2024
Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 0

Configuration

Squadron Mode: essential

Commits Reviewed

caf4bd7c1eec9defb0ef8baba7bb9835b58a74db...a3e1bc5e9b94c2c3444bc19196e4bc3ebebf2847

Files Reviewed
  • packages/runtime/src/plugins/generate-preload-assets.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 6

Configuration

Squadron Mode: essential

Commits Reviewed

44ac346ef4212a17e4b2d611ba0e7948ba804947...dd254ef53f13419cdb9292cc3a35103d3cc0c2cc

Files Reviewed
  • packages/runtime/src/plugins/generate-preload-assets.ts
  • packages/runtime/src/utils/load.ts
  • packages/sdk/src/dom.ts
  • packages/sdk/src/generateSnapshotFromManifest.ts
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

@@ -328,7 +328,7 @@ export const generatePreloadAssetsPlugin: () => FederationRuntimePlugin =
moduleInfo: {
name: remoteInfo.name,
entry: remote.entry,
type: 'global',
type: remoteInfo.type || 'global',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider providing a default value for the type property to ensure it always has a valid value, even if remoteInfo.type is undefined:

Suggested change
type: remoteInfo.type || 'global',
type: remoteInfo.type || 'global',

This change ensures that the type property will always have a value, defaulting to 'global' if remoteInfo.type is not provided. This can help prevent potential issues down the line if the code assumes type is always defined.

@module-federation module-federation deleted a comment from squadronai bot Sep 26, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 26, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 26, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 26, 2024
TextEncoder,
console,
require: eval('require'),
__dirname,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bundler will transform these into strings in other targets, like browser etc. cant use short hand

Suggested change
__dirname,
__dirname: __dirname,
__filename: __filename,

const code = await response.text();

const module: any = new vm.SourceTextModule(code, {
context,
Copy link
Member

@ScriptedAlchemy ScriptedAlchemy Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think you should pass a custom VM context to the module. Attempting to copy the global context does not work for many cases, there are many more primitives than just

  Event,
      URL,
      URLSearchParams,
      TextDecoder,
      TextEncoder,
      console,

if you pass no context, then it will run in the current context and should operate like runInThisContext does for Script

Copy link
Contributor

@squadronai squadronai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental Review

Comments posted: 3

Configuration

Squadron Mode: essential

Commits Reviewed

2042777a4f2bfbb66d56f7cb08d94405dbe30048...7c2df68b4b924be1c8d19944687955915d047b2f

Files Reviewed
  • packages/runtime/src/plugins/generate-preload-assets.ts
  • packages/runtime/src/utils/load.ts
  • packages/sdk/src/dom.ts
  • packages/sdk/src/generateSnapshotFromManifest.ts
  • packages/sdk/src/node.ts
Files Ignored

These files were ignored due to the filter in the squadron.yaml file.

  • package.json

@module-federation module-federation deleted a comment from squadronai bot Sep 28, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 28, 2024
@module-federation module-federation deleted a comment from squadronai bot Sep 28, 2024
@ScriptedAlchemy ScriptedAlchemy changed the base branch from main to feature/node-esm September 28, 2024 21:59
@ScriptedAlchemy
Copy link
Member

Moving to internal branch to help

1 similar comment
@ScriptedAlchemy
Copy link
Member

Moving to internal branch to help

@ScriptedAlchemy ScriptedAlchemy merged commit c529a72 into module-federation:feature/node-esm Sep 28, 2024
13 of 15 checks passed
ScriptedAlchemy added a commit that referenced this pull request Sep 29, 2024
Co-authored-by: zhn <zhang_h_n@163.com>
Co-authored-by: 张洪恩 <zhanghongen@bwcj.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants