Skip to content
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

feat(plugin): support for esm loader which its ext is .js in rspack #393

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

easy1090
Copy link
Contributor

Summary

feat(plugin): support for esm loader which its ext is .js

Related Links

closes: #330

Copy link

netlify bot commented Jun 27, 2024

Deploy Preview for rsdoctor ready!

Name Link
🔨 Latest commit 7af081f
🔍 Latest deploy log https://app.netlify.com/sites/rsdoctor/deploys/667d0e64f7c5030008ac6483
😎 Deploy Preview https://deploy-preview-393--rsdoctor.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@easy1090 easy1090 requested a review from chenjiahan June 27, 2024 05:46
@easy1090 easy1090 changed the title feat(plugin): support for esm loader which its ext is .js feat(plugin): support for esm loader which its ext is .js in rspack Jun 27, 2024
if (dirname(current) === current) {
break;
}
current = dirname(current);
Copy link
Member

Choose a reason for hiding this comment

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

dirname(current) is called twice, which can be stored in a variable

return;
}

// Some packages will put an empty package.json in the source folder.
Copy link
Member

Choose a reason for hiding this comment

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

I thought Node.js will also consider empty package.json as a valid package.json file, should we skip it?

Copy link
Contributor Author

@easy1090 easy1090 Jun 27, 2024

Choose a reason for hiding this comment

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

这里不可以视为 valid,否则会导致重复包计算准确,很多 ui 库会在组件文件夹下留个 package.json 设置 sideEffect 属性且没有 name。这种情况就要继续递归向上查找。

Copy link
Member

Choose a reason for hiding this comment

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

For Node.js type="module", the empty package.json should not be skipped.

If we want to skip package.json for component libraries, maybe we can add a param to control it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

create a new issue to fix this case:#394

feat(plugin): support for esm loader which its ext is .js
@easy1090 easy1090 force-pushed the feat/esm-loader-optimize-2 branch from 1757369 to 7af081f Compare June 27, 2024 07:01
@easy1090 easy1090 merged commit f413761 into main Jun 27, 2024
8 checks passed
@easy1090 easy1090 deleted the feat/esm-loader-optimize-2 branch June 27, 2024 07:12
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.

2 participants