Skip to content

fix(rspack): only patchChunkSplit when config set exposes#3095

Merged
2heal1 merged 1 commit intomainfrom
fix/patch-chunk-split
Oct 28, 2024
Merged

fix(rspack): only patchChunkSplit when config set exposes#3095
2heal1 merged 1 commit intomainfrom
fix/patch-chunk-split

Conversation

@2heal1
Copy link
Copy Markdown
Member

@2heal1 2heal1 commented Oct 21, 2024

Description

only patchChunkSplit when config set exposes

Related Issue

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.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Oct 21, 2024

🦋 Changeset detected

Latest commit: 83c3f44

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 39 packages
Name Type
@module-federation/rspack Patch
@module-federation/enhanced Patch
@module-federation/modern-js Patch
@module-federation/nextjs-mf Patch
@module-federation/node Patch
@module-federation/rsbuild-plugin Patch
3008-runtime-remote Patch
host Patch
host-v5 Patch
host-vue3 Patch
@module-federation/modernjs Patch
modernjs-ssr-dynamic-nested-remote Patch
modernjs-ssr-dynamic-remote-new-version Patch
modernjs-ssr-dynamic-remote Patch
modernjs-ssr-host Patch
modernjs-ssr-nested-remote Patch
modernjs-ssr-remote-new-version Patch
modernjs-ssr-remote Patch
remote1 Patch
remote2 Patch
remote3 Patch
remote4 Patch
@module-federation/runtime Patch
@module-federation/webpack-bundler-runtime Patch
@module-federation/sdk Patch
@module-federation/runtime-tools Patch
@module-federation/managers Patch
@module-federation/manifest Patch
@module-federation/dts-plugin Patch
@module-federation/third-party-dts-extractor Patch
@module-federation/devtools Patch
@module-federation/bridge-react Patch
@module-federation/bridge-vue3 Patch
@module-federation/bridge-shared Patch
@module-federation/bridge-react-webpack-plugin Patch
@module-federation/retry-plugin Patch
@module-federation/data-prefetch Patch
@module-federation/esbuild Patch
@module-federation/utilities Patch

Not sure what this means? Click here to learn what changesets are.

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

@netlify
Copy link
Copy Markdown

netlify bot commented Oct 21, 2024

Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 83c3f44
🔍 Latest deploy log https://app.netlify.com/sites/module-federation-docs/deploys/671614fc4fca290008ca91e1
😎 Deploy Preview https://deploy-preview-3095--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
Copy Markdown
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

The pull request fixes an issue in the rspack package's ModuleFederationPlugin.ts file. The changes ensure that the _patchChunkSplit method is only applied when the containerManager.enable flag is set. Additionally, the containerManager.init method is moved to be called before the _patchChunkSplit method, and the containerManager.containerPluginExposesOptions is assigned to the options.exposes property. These changes help to improve the integration of the Module Federation functionality with the existing codebase.

File Summaries
File Summary
packages/rspack/src/ModuleFederationPlugin.ts The code changes introduce a conditional check to only apply the _patchChunkSplit method when the containerManager.enable flag is set. Additionally, the containerManager.init method is moved to be called before the _patchChunkSplit method, and the containerManager.containerPluginExposesOptions is assigned to the options.exposes property.

Copy link
Copy Markdown
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: 1

Configuration

Squadron Mode: essential

Commits Reviewed

cf14509062fd942b5763e8c757b4eb07a865611f...83c3f446e8176fda4dfd3340b277ea0449b5c30b

Files Reviewed
  • packages/rspack/src/ModuleFederationPlugin.ts
Files Ignored

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

  • .changeset/tricky-sheep-travel.md

Comment on lines +73 to +75
if (containerManager.enable) {
this._patchChunkSplit(compiler, options.name);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider extracting the condition into a descriptive variable for better readability:

Suggested change
if (containerManager.enable) {
this._patchChunkSplit(compiler, options.name);
}
const shouldPatchChunkSplit = containerManager.enable && options.exposes;
if (shouldPatchChunkSplit) {
this._patchChunkSplit(compiler, options.name);
}

This change makes the intention clearer and allows for easier expansion of the condition if needed in the future.

@2heal1 2heal1 merged commit 86b4cdc into main Oct 28, 2024
@2heal1 2heal1 deleted the fix/patch-chunk-split branch October 28, 2024 09:44
@2heal1 2heal1 mentioned this pull request Oct 30, 2024
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.

2 participants