Skip to content

test: more case about cjs bundle to esm lib #19787

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

Merged
merged 3 commits into from
Aug 12, 2025
Merged

Conversation

hai-x
Copy link
Member

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

What kind of change does this PR introduce?

Add more test case.

Did you add tests for your changes?

Yes, feel free to more requirement

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 11, 2025

CodSpeed Performance Report

Merging #19787 will degrade performances by 63.1%

Comparing more-cjs-case (6e567a8) with main (ad23854)

Summary

⚡ 3 improvements
❌ 2 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}' 95.9 ms 44.7 ms ×2.1
benchmark "devtool-source-map", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 144.4 ms 86.3 ms +67.24%
benchmark "future-defaults", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 51.8 ms 88.5 ms -41.53%
benchmark "many-chunks-esm", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 136.8 ms 370.8 ms -63.1%
benchmark "many-modules-esm", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 408 ms 114.8 ms ×3.6

@hai-x hai-x marked this pull request as ready for review August 11, 2025 17:23
@@ -0,0 +1,3 @@
console.log(exports)
console.log(module)
console.log(this)
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid using console.log, it outputs a lot of span into CI output, ideally we should remove console.log and debugger from all tests (except where we need it)


module.exports = {
name: "overrides-exports-cjs"
};
Copy link
Member

Choose a reason for hiding this comment

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

let's add a test case:

module.exports = {
	name: "adding-exports-cjs"
};

module.exports.foo = "foo";

expect(lib3.name).toBe("overrides-exports-cjs");
expect(lib3.foo).toBe(undefined);
expect(lib4).toMatchObject({});
expect(lib5).toMatchObject({});
Copy link
Member

Choose a reason for hiding this comment

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

Is it expected, because I think we should have adding-exports-cjs and foo

@alexander-akait alexander-akait merged commit 61a15a6 into main Aug 12, 2025
78 of 82 checks passed
@alexander-akait alexander-akait deleted the more-cjs-case branch August 12, 2025 19:14
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.

2 participants