fix(css): ensure code is valid after empty css chunk imports are removed (fix #14515) #14517
Conversation
There was a problem hiding this comment.
(I really would like to add test cases, but it seems pretty hard to mock the plugin state for only testing the output hook)
We can maybe refactor this empty chunk replacement logic out as a function to unit test and create snapshots. That we can easily compare how this PR affects it. It may be best to do this step in another PR first so we get a starting snapshot.
susnux
force-pushed
the
fix/css-import-removal-with-minify
branch
from
3842602
to
1ad178e
Compare
Make sure that minified code that chains `require` calls with the `,` operator will not result in invalid code when removing one of the chained requires. Previously the last require that terminates with a semicolon was removed with the semicolon. This produced invalid code like `require(), /* empty css */const foo =` Now we append either a comma if a next require call was chained, like `require(...), require(css), other` --> `require(...)/* empty css */, other` Or terminate with a semicolon `require(...), require(css); const foo = ...` --> `require(...)/* empty css */; const foo = ...` Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
susnux
force-pushed
the
fix/css-import-removal-with-minify
branch
from
1ad178e
to
3214fa4
Compare
@bluwy Tests updated, you can compare it now :) |
bluwy
added
feat: css
p3-minor-bug
I used vite@5.0.0-beta.15 and it is still not working. I look this problem up in the changelog, and find that it was fixed in an earlier version beta.13
const e=,/* empty css */ // ❎wrong
const e=/* empty css */, // ✅right',' should be put behind |
|
@emosheeep I don't think moving the comma to behind will work. It will still be a parse error. Is there a reason the code is Is this only happening in the Vite beta? I'd suggest opening a new issue with a repro so we could look into it. |
|
emmm, sorry I can hardly provide...I encountered this in my company project, which is complex very much. But the most difference usage I think about is that there are so many entries added to 
And if I remove extra entries, only preserve necessary entries, everything runs well. |
Description
Make sure that minified code that chains
requirecalls with the,operator will not result in invalid code when removing one of the chained requires. Previously the last require that terminates with a semicolon was removed with the semicolon. This produced invalid code likerequire(), /* empty css */const foo =Now we append either a comma if a next require call was chained, like
require(...), require(css), other-->require(...)/* empty css */, otherOr terminate with a semicolon
require(...), require(css); const foo = ...-->require(...)/* empty css */; const foo = ...Additional context
This also fixes a problem where empty chunks were not removed if chained in the middle with
,.E.g. this like this:
require(...), require(empty chunk), require(...);(I really would like to add test cases, but it seems pretty hard to mock the plugin state for only testing the output hook)
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).