Skip to content

SideEffectsPlugin don't modify cache entries - #7408

Merged
sokra merged 3 commits into
masterfrom
bugfix/side-effects-caching
May 28, 2018
Merged

sokra merged 3 commits into
masterfrom
bugfix/side-effects-caching

Conversation

@sokra

@sokra sokra commented May 26, 2018

Copy link
Copy Markdown
Member

fix bug where modules are incorrectly including in chunks

fixes #7400

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
yes

Does this PR introduce a breaking change?
yes, but its a bugfix

What needs to be documented once your changes are merged?
nothing

@webpack-bot

webpack-bot commented May 26, 2018

Copy link
Copy Markdown
Contributor

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

@sokra
sokra force-pushed the bugfix/side-effects-caching branch from 0681fe6 to a297c48 Compare May 27, 2018 19:50
@sokra
sokra force-pushed the bugfix/side-effects-caching branch from a297c48 to ae8d674 Compare May 27, 2018 19:53
@webpack-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

for (const chunk of compilation.chunks) {
if (chunk.entryModule) {
processModule(chunk.entryModule, true);
for (const preparedEntrypoint of compilation._preparedEntrypoints) {

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.

It is possible to use compilation.entrypoints instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sadly not, as this plugin runs before these property is set. But we should make _preparedEntrypoints public in the next major version

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.

Thanks for clarification. Since _preparedEntrypoints is a private API, isn't it safe to change it now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Technically yes, but I better don't risk it...

@sokra
sokra merged commit 1f2584e into master May 28, 2018
@sokra
sokra deleted the bugfix/side-effects-caching branch May 28, 2018 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recompilation in development mode generates incorrect imports

3 participants