Skip to content

Conversation

@hai-x
Copy link
Contributor

@hai-x hai-x commented Feb 19, 2025

Summary

Fixes web-infra-dev/rslib#714 (comment)

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@netlify
Copy link

netlify bot commented Feb 19, 2025

Deploy Preview for rspack canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 22a212a
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/67bdf65fa407640008da73f9

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 19, 2025

CodSpeed Performance Report

Merging #9390 will not alter performance

Comparing hai-x:fix-cjs-statis (22a212a) with main (ba7556d)

🎉 Hooray! codspeed-node just leveled up to 4.0.0!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 7 untouched benchmarks

@hai-x hai-x changed the title [WIP] fix: commonjs static library mode fix: commonjs static library mode Feb 19, 2025
@hai-x hai-x marked this pull request as ready for review February 19, 2025 14:56
@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Feb 19, 2025
@hai-x
Copy link
Contributor Author

hai-x commented Feb 24, 2025

@ahabhgk Could you help review it? Thanks.

Copy link
Member

@fi3ework fi3ework left a comment

Choose a reason for hiding this comment

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

This test case for commonjs-static is supposed to be uncommented here.

// TODO: amd esm import exports presence check
// {
// resolve: {
// alias: {
// library: path.resolve(
// testPath,
// "../0-create-library/commonjs-static-external.js"
// ),
// external: path.resolve(__dirname, "node_modules/external.js")
// }
// },
// plugins: [
// new webpack.DefinePlugin({
// NAME: JSON.stringify("commonjs-static with external"),
// TEST_EXTERNAL: true
// })
// ]
// },

@hai-x
Copy link
Contributor Author

hai-x commented Feb 24, 2025

This test case for commonjs-static is supposed to be uncommented here.

// TODO: amd esm import exports presence check
// {
// resolve: {
// alias: {
// library: path.resolve(
// testPath,
// "../0-create-library/commonjs-static-external.js"
// ),
// external: path.resolve(__dirname, "node_modules/external.js")
// }
// },
// plugins: [
// new webpack.DefinePlugin({
// NAME: JSON.stringify("commonjs-static with external"),
// TEST_EXTERNAL: true
// })
// ]
// },

Ok. I will uncomment it.

@hai-x
Copy link
Contributor Author

hai-x commented Feb 25, 2025

Done.

@hai-x hai-x requested review from ahabhgk and fi3ework February 25, 2025 16:59
Copy link
Contributor

@ahabhgk ahabhgk 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

@ahabhgk ahabhgk merged commit 9cf027f into web-infra-dev:main Feb 26, 2025
27 checks passed
@hai-x hai-x deleted the fix-cjs-statis branch February 26, 2025 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: No commonjs export names annotations generated

4 participants