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

fix: avoid shared module will be compiled in wrong target compiler #1996

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

Conversation

2heal1
Copy link
Member

@2heal1 2heal1 commented Jan 19, 2024

Description

  • avoid shared module will be compiled in wrong target compiler

html-webpack-plugin will use childCompiler which loader target is node to compile . And it will output the asset if the shared set a file path like this:

shared: {
  'shared-config': {
     import: './src/shared-config'
    }
},

html-webpack-plugin asset:

image

default target(browser) asset:

image

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.
  • [x ] I have updated the documentation.

@2heal1
Copy link
Member Author

2heal1 commented Jan 19, 2024

@ScriptedAlchemy I'm not sure whether the ProvideSharedPlugin can use thisCompilation instead of compilation, please help to review and check

@2heal1
Copy link
Member Author

2heal1 commented Jan 19, 2024

the webpack reproduce demo:

  1. pnpm i
  2. pnpm build
  3. watch the dist , and you will see two same name but different hash files :

image

One is bundled from htmlPlugin childCompiler and the other is from main compiler

@ScriptedAlchemy
Copy link
Member

I'm not sure of the implications of doing this. Would it mean that any child compilers created would not include any federation stuff unless the plugin is applied again on the child compiler?

@ScriptedAlchemy
Copy link
Member

Can we cut a release of this? then i can override the packages in federation examples and we can test against all those as well

@2heal1
Copy link
Member Author

2heal1 commented Jan 22, 2024

I'm not sure of the implications of doing this. Would it mean that any child compilers created would not include any federation stuff unless the plugin is applied again on the child compiler?

@ScriptedAlchemy Yes, because the child compiler may have different target than the main compiler. Like HtmlPlugin sub cpmpiler will have node target, but the main compiler have browser target .

But maybe we can solve it according another way .

@2heal1
Copy link
Member Author

2heal1 commented Jan 22, 2024

the webpack reproduce demo:

  1. pnpm i
  2. pnpm build
  3. watch the dist , and you will see two same name but different hash files :

image

One is bundled from htmlPlugin childCompiler and the other is from main compiler

@ScriptedAlchemy Can you help to check this demo ? Maybe we can solve the issue in a simple way.

@ahabhgk
Copy link

ahabhgk commented Jan 22, 2024

This is caused by addInclude in ProvideSharedPlugin, all provide modules will be added to compilation (and also child compilations) at hooks.finishMake.

Actually the additional html-webpack-plugin asset won't affect anything, and won't cause any issue, only the output size is bigger, I think it's ok to have these additional html-webpack-plugin assets.
From the perspective of impact, can't shared modules in child compilation has more impact, it's potentially a breaking change, so I suggest keep it as it is

@ahabhgk
Copy link

ahabhgk commented Jan 22, 2024

Change htmlWebpackPlugin from child compiler to compilation.executeModule seems a better way to solve this

@2heal1
Copy link
Member Author

2heal1 commented Jan 22, 2024

This is caused by addInclude in ProvideSharedPlugin, all provide modules will be added to compilation (and also child compilations) at hooks.finishMake.

Actually the additional html-webpack-plugin asset won't affect anything, and won't cause any issue, only the output size is bigger, I think it's ok to have these additional html-webpack-plugin assets. From the perspective of impact, can't shared modules in child compilation has more impact, it's potentially a breaking change, so I suggest keep it as it is

@ahabhgk Yes, it will not have issues in most cases . But if users set chunkFileName as [name].js, which not include hash , the compile will be failed because of duplicate name file.
image

@2heal1
Copy link
Member Author

2heal1 commented Jan 22, 2024

Change htmlWebpackPlugin from child compiler to compilation.executeModule seems a better way to solve this

@ahabhgk how to set compilation.executeModule ? Do you mean excludeChunks in htmlWebpackPlugin ? I try it and not work as expected.

@ahabhgk
Copy link

ahabhgk commented Jan 22, 2024

HtmlWebpackPlugin use child compiler to compile and execute to get the html asset, importModule (executeModule) is an alternative for child compiler, MiniCssExtractPlugin already switch to importModule from child compiler, HtmlWebpackPlugin hasn't, maybe we can have a try and contribute to HtmlWebpackPlugin

@ahabhgk
Copy link

ahabhgk commented Jan 24, 2024

One thing that wired to me is ConsumeSharedPlugin is using hooks.thisCompilation instead of hooks.compilation, what's the point if you provide shared modules in child compilation but only consume it in the main compilation. Now this fix make since to me :D

Copy link
Contributor

Stale pull request message

Copy link
Contributor

Stale pull request message

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.

None yet

3 participants