fix(css): ensure code is valid after empty css chunk imports are removed (fix #14515)

[susnux]

Description

• Resolves [css] Removing empty css causes invalid code to be generated #14515

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 = ...

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?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[bluwy]

(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]

@bluwy

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.

Done here: #14528

[susnux]

Hmm I guess that's fair too. We'll compare this change in the snapshots then if it's fine.

@bluwy Tests updated, you can compare it now :)

[bluwy]

Thanks! Yes it looks a lot easier to review now.

[emosheeep]

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

[bluwy]

@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 require-ing CSS and assigning the value of it to the e variable?

Is this only happening in the Vite beta? I'd suggest opening a new issue with a repro so we could look into it.

[emosheeep]

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 build.lib.entries (about 600+), and the problem only exists in cjs file, the esm output is normal.

And if I remove extra entries, only preserve necessary entries, everything runs well.