Skip to content

fix: should avoid through variables in inlined module#18845

Merged
alexander-akait merged 1 commit intowebpack:mainfrom
fi3ework:avoid-inline-through-var-dev
Oct 14, 2024
Merged

fix: should avoid through variables in inlined module#18845
alexander-akait merged 1 commit intowebpack:mainfrom
fi3ework:avoid-inline-through-var-dev

Conversation

@fi3ework
Copy link
Copy Markdown
Contributor

@fi3ework fi3ework commented Oct 10, 2024

What kind of change does this PR introduce?

What does it fix:

Changes overview:

  • Reuse the renaming logic from lib/optimize/ConcatenatedModule.js in lib/javascript/JavascriptModulesPlugin.js
  • Extract reusable functions to lib/util/concatenate.js from lib/optimize/ConcatenatedModule.js and remove the duplicated in lib/javascript/JavascriptModulesPlugin.js
  • Adding Synmbol as a js globals toRESERVED_NAMES.

Did you add tests for your changes?

Yes.

Does this PR introduce a breaking change?

No, but there's an API findNewName is removed from lib/javascript/JavascriptModulesPlugin.js, do we need to preserve that? I'm not clear about are the methods of plugins are treated as the stable stuff. If so, we could simply proxy the refactored method.

What needs to be documented once your changes are merged?

No.

@webpack-bot
Copy link
Copy Markdown
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

Copy link
Copy Markdown
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Looks good, after tests, we can merge

@fi3ework
Copy link
Copy Markdown
Contributor Author

fi3ework commented Oct 11, 2024

@alexander-akait I'll add the related test and clean code tonight (in about 10 hours), after that, we're good to merge.

@webpack-bot
Copy link
Copy Markdown
Contributor

@fi3ework Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@alexander-akait Please review the new changes.

@fi3ework fi3ework force-pushed the avoid-inline-through-var-dev branch from a41d7d8 to bb10e4b Compare October 11, 2024 18:27
@fi3ework fi3ework marked this pull request as ready for review October 11, 2024 18:51
Copy link
Copy Markdown
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@alexander-akait
Copy link
Copy Markdown
Member

@fi3ework Is it ready to merge?

@fi3ework
Copy link
Copy Markdown
Contributor Author

@alexander-akait Yes, it's ready for review.

@alexander-akait alexander-akait merged commit 18d6cb5 into webpack:main Oct 14, 2024
@alexander-akait
Copy link
Copy Markdown
Member

Thank you

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.

Collision with global variables when optimization.avoidEntryIife = true

4 participants