feat(CommonJsPlugin): magic comment to ignore require calls #11316

petermetz · 2020-08-12T23:02:44Z

A partial port of the same magic comments that are supported
for import() calls already via this commit:
f65e9da

Signed-off-by: Peter Somogyvari peter.metz@unarin.com

Motivation is in the issue comments that I linked above. Bottom line is that I'm using Typescript+Webpack+NodeJS and the import() calls get transpiled into require so the import ignore feature doesn't work for me I need the same for require() calls.

What kind of change does this PR introduce?

feature

Did you add tests for your changes?

Yes.

Does this PR introduce a breaking change?

Not that I'm aware of. It's a new feature.

What needs to be documented once your changes are merged?

You can enable magic comment for CommonJs with module.parser.javascript.commonjsMagicComments: true and in Rule.parser.commonjsMagicComments: true.
With that enabled you can use the webpackIgnore: true magic comment for require calls as well: https://webpack.js.org/api/module-methods/#commonjs

Some configuration options has been moved to parser objects as well:

module.{expr|wrapped|unknown}Context{Request|RegExp|Recursive|Critical}
module.strictExportPresence
module.strictThisContextOnImports
All have been moved to module.parser.javascript.xxx resp. Rule.parser.xxx
If that's not already in the docs: To get the parser options of a module the options in module.parser.[module-type] are merged with Rule.parser. The same for the generator options.
- If the module type contains / the base module type options are also merged. e.g. for javascript/esm: module.parser.javascript + module.parser["javascript/esm"] + Rule.parser (could be multiple, if multiple rules match).

jsf-clabot · 2020-08-12T23:02:46Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

webpack-bot · 2020-08-12T23:02:48Z

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)

petermetz · 2020-08-12T23:08:49Z

Note: It would be great if this could be back-ported to 4.x since AFAIK 5 is not out/stable yet.

anshumanv · 2020-08-13T04:38:31Z

Note: It would be great if this could be back-ported to 4.x since AFAIK 5 is not out/stable yet.

Will take care of the backport 👍

sokra · 2020-08-13T04:47:12Z

Looks good. I wonder how this affects performance considering there are a lot more require()s compare to import()s in a codebase

petermetz · 2020-08-13T16:37:55Z

Looks good. I wonder how this affects performance considering there are a lot more require()s compare to import()s in a codebase

@sokra It depends mostly on the performance characteristics of the comment parsing in the edge case when there are no comments to be parsed in the require() call (which is what will happen 99.9% of the time).
My guess is that the parser is already reasonably well optimized because it's such a central component, but you tell me.
If it turns out that it does cause a significant slowdown we could add a flag in the plugin's options that stops the parsing from happening unless explicitly enabled by that flag. Then the documentation can say that first you need to enable the plugin flag in the webpack config file and then you can start adding the magic comments in the code.

sokra · 2020-08-13T18:10:18Z

Yeah actually the getComments function in JavascriptParser is a bit inefficient.

Could you improve it by using binary search? The comments array should be sorted anyway...

petermetz · 2020-08-13T22:22:25Z

Yeah actually the getComments function in JavascriptParser is a bit inefficient.

Could you improve it by using binary search? The comments array should be sorted anyway...

@sokra Would something like this be okay? => 04d6b0f

I left it in a separate commit because it doesn't really have anything to do with the magic comment feature itself, but happy to squash of course, just let me know.

EDIT: Updated commit link after force push

webpack-bot · 2020-08-13T23:11:53Z

Hi @petermetz.

Just a little hint from a friendly bot about the best practice when submitting pull requests:

Don't submit pull request from your own master branch. It's recommended to create a feature branch for the PR.

You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR.

lib/util/binarySearch.js

lib/javascript/JavascriptParser.js

lib/util/binarySearch.js

webpack-bot · 2020-08-14T19:24:41Z

@petermetz Thanks for your update.

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

@sokra Please review the new changes.

Depends on webpack/webpack#11316 This has no real effect until the PR above is merged and releaed on Webpack (and then we upgrade to that release for the builds) How this fix works is explained in the linked PR in greater detail. Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>

alexander-akait · 2021-01-19T10:42:50Z

@petermetz Sorry for delay, can you rebase?

A partial port of the same magic comments that are supported for import() calls already via this commit: webpack@f65e9da Signed-off-by: Peter Somogyvari <peter.metz@unarin.com>

Signed-off-by: Peter Somogyvari <peter.metz@unarin.com>

petermetz · 2021-01-19T19:57:03Z

@petermetz Sorry for delay, can you rebase?

@alexander-akait No worries at all. I did the rebase just now, enjoy!

move javascript related options from 'module' to 'module.parser.javascript'

webpack-bot · 2021-01-21T08:01:01Z

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

sokra · 2021-01-21T08:01:26Z

Thanks

webpack-bot · 2021-01-22T13:41:35Z

I've created an issue to document this in webpack/webpack.js.org.

webpack-bot added the PR: unreviewed label Aug 12, 2020

petermetz mentioned this pull request Aug 12, 2020

webpackIgnore: true is ignored, dynamic import/require still processed #11311

Closed

webpack-bot added PR: CI-ok PR: tests-needed labels Aug 12, 2020

webpack-bot added PR: CI-not-ok and removed PR: CI-ok labels Aug 13, 2020

petermetz force-pushed the master branch from 12b9a32 to 04d6b0f Compare August 14, 2020 02:31

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Aug 14, 2020

alexander-akait reviewed Aug 14, 2020

View reviewed changes

lib/util/binarySearch.js Outdated Show resolved Hide resolved

sokra requested changes Aug 14, 2020

View reviewed changes

lib/javascript/JavascriptParser.js Outdated Show resolved Hide resolved

lib/util/binarySearch.js Outdated Show resolved Hide resolved

webpack-bot added PR: reviewed-changes-requested PR: review-outdated and removed PR: unreviewed PR: reviewed-changes-requested labels Aug 14, 2020

webpack-bot added PR: CI-not-ok and removed PR: CI-ok labels Aug 14, 2020

petermetz force-pushed the master branch 3 times, most recently from 5fa8e15 to 3790c0b Compare August 14, 2020 22:35

petermetz force-pushed the master branch from bae319a to a1021fd Compare November 7, 2020 02:13

petermetz added 2 commits January 19, 2021 11:54

feat(CommonJsPlugin): magic comment to ignore require calls

12f37fe

A partial port of the same magic comments that are supported for import() calls already via this commit: webpack@f65e9da Signed-off-by: Peter Somogyvari <peter.metz@unarin.com>

perf(parser): binary search for comments in range

ded4cc6

Signed-off-by: Peter Somogyvari <peter.metz@unarin.com>

petermetz force-pushed the master branch from a1021fd to ded4cc6 Compare January 19, 2021 19:55

sort imports

be13a46

webpack-bot added PR: CI-not-ok and removed PR: CI-ok labels Jan 20, 2021

add commonjsMagicComments options

013ca3b

move javascript related options from 'module' to 'module.parser.javascript'

sokra force-pushed the master branch from 0ebf663 to 013ca3b Compare January 20, 2021 19:20

sokra approved these changes Jan 20, 2021

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels Jan 20, 2021

remove duplication in schema and types

7002681

sokra approved these changes Jan 21, 2021

View reviewed changes

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Jan 21, 2021

sokra closed this Jan 21, 2021

sokra reopened this Jan 21, 2021

sokra merged commit a07a126 into webpack:master Jan 21, 2021

webpack-bot mentioned this pull request Jan 22, 2021

Document webpack change: feat(CommonJsPlugin): magic comment to ignore require calls webpack/webpack.js.org#4470

Closed

alexander-akait mentioned this pull request Feb 1, 2021

Using dynamic require on node targets WITHOUT resolve or bundle the target module #4175

Closed

vivaxy mentioned this pull request Mar 25, 2021

fix(commitlint): 🐛 repair commitlint configuration failures vivaxy/vscode-conventional-commits#103

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(CommonJsPlugin): magic comment to ignore require calls #11316

feat(CommonJsPlugin): magic comment to ignore require calls #11316

petermetz commented Aug 12, 2020 •

edited by sokra

Loading

jsf-clabot commented Aug 12, 2020 •

edited

Loading

webpack-bot commented Aug 12, 2020 •

edited by sokra

Loading

petermetz commented Aug 12, 2020

anshumanv commented Aug 13, 2020

sokra commented Aug 13, 2020 •

edited

Loading

petermetz commented Aug 13, 2020

sokra commented Aug 13, 2020

petermetz commented Aug 13, 2020 •

edited

Loading

webpack-bot commented Aug 13, 2020

webpack-bot commented Aug 14, 2020

alexander-akait commented Jan 19, 2021

petermetz commented Jan 19, 2021

webpack-bot commented Jan 21, 2021

sokra commented Jan 21, 2021

webpack-bot commented Jan 22, 2021

feat(CommonJsPlugin): magic comment to ignore require calls #11316

feat(CommonJsPlugin): magic comment to ignore require calls #11316

Conversation

petermetz commented Aug 12, 2020 • edited by sokra Loading

jsf-clabot commented Aug 12, 2020 • edited Loading

webpack-bot commented Aug 12, 2020 • edited by sokra Loading

petermetz commented Aug 12, 2020

anshumanv commented Aug 13, 2020

sokra commented Aug 13, 2020 • edited Loading

petermetz commented Aug 13, 2020

sokra commented Aug 13, 2020

petermetz commented Aug 13, 2020 • edited Loading

webpack-bot commented Aug 13, 2020

webpack-bot commented Aug 14, 2020

alexander-akait commented Jan 19, 2021

petermetz commented Jan 19, 2021

webpack-bot commented Jan 21, 2021

sokra commented Jan 21, 2021

webpack-bot commented Jan 22, 2021

petermetz commented Aug 12, 2020 •

edited by sokra

Loading

jsf-clabot commented Aug 12, 2020 •

edited

Loading

webpack-bot commented Aug 12, 2020 •

edited by sokra

Loading

sokra commented Aug 13, 2020 •

edited

Loading

petermetz commented Aug 13, 2020 •

edited

Loading