-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix(optimize-deps): don't externalize JS files imported with asset extensions #16242
fix(optimize-deps): don't externalize JS files imported with asset extensions #16242
Conversation
Run & review this pull request in StackBlitz Codeflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Would you add a test case in playground/optimize-deps
?
sure, I'll add a test case |
@sapphi-red hi, I have submitted the test code, but how does the Actions check failing |
I guess you forgot to run |
if (kind === 'require-call') { | ||
// #16116 fix: Import the module.scss path, which is actually module.scss.js | ||
if (resolved.endsWith('.js')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we only check this in require-call
only? Wouldn't this help for imports too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reason is because we don't support import
without extensions in dependencies. But I've noticed that there's a case like import 'normalize.css'
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, you have to specify the full path with imports. Let's try this then.
Description
fix #16116 importing a CommonJS module installed from NPM. The modue contains files with filenames like PeoplePicker.scss.js. These are JavaScript files, obviously, but the filename contains scss.
What is the purpose of this pull request?