Skip to content

fix: needCreateRequire should be set to true only when module output and node target #19761

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hai-x
Copy link
Member

@hai-x hai-x commented Aug 3, 2025

What kind of change does this PR introduce?

Fixes #19441.

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

No

Copy link

codspeed-hq bot commented Aug 3, 2025

CodSpeed Performance Report

Merging #19761 will degrade performances by 94.03%

Comparing non_webpack_require (e074c12) with main (f2228ea)

Summary

⚡ 2 improvements
❌ 3 regressions
✅ 37 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
benchmark "devtool-eval", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 50 ms 105.9 ms -52.79%
benchmark "devtool-source-map", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 102.7 ms 69.8 ms +47.1%
benchmark "lodash", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 27.2 ms 60.3 ms -54.93%
benchmark "many-chunks-esm", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 111 ms 45.7 ms ×2.4
benchmark "three-long", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 299.5 ms 5,018.3 ms -94.03%

@hai-x hai-x force-pushed the non_webpack_require branch 2 times, most recently from 4c95d99 to 4026dcd Compare August 3, 2025 18:40
@hai-x hai-x changed the title fix: warn about __non_webpack_require__ when module output but target is not node fix: needCreateRequire should be set to true only when module output and node target Aug 3, 2025
@hai-x hai-x force-pushed the non_webpack_require branch from 4026dcd to 97499fb Compare August 3, 2025 19:13
Copy link
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.

Tests are failed

@hai-x hai-x force-pushed the non_webpack_require branch from 97499fb to de29b5e Compare August 4, 2025 15:34
lib/APIPlugin.js Outdated
importMetaName
);
const moduleOutput = compilation.options.output.module;
const nodeTarget = compilation.options.externalsPresets.node;
Copy link
Member

Choose a reason for hiding this comment

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

Better to use compiler.platform.node to check is it node target or not, don't forget we have target: ['web', 'node'] (i.e. universal target, it is not fully ready right now, but we should finish it soon)

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.

3 participants