fix: avoid css leaking into emitted javascript

[ferdinando-ferreira]

Description

Even with styles being emitted to their own .css files, their source code is present into the emitted .js files (both in regular and ssr modes) into a variable that is never used in the resulting source code.

It appears that this was added to prevent the css module from being "tree shaken" out of the compilation in this point of code so it can be used afterwards in the renderChunk hook.

vite/packages/vite/src/node/plugins/css.ts

Lines 166 to 175 in 4a955bc

	if (!code) {
	if (config.build.cssCodeSplit) {
	// If code-splitting CSS, inject a fake marker to avoid the module
	// from being tree-shaken. This preserves the .css file as a
	// module in the chunk's metadata so that we can retrieve them in
	// renderChunk.
	code += `${cssInjectionMarker}()\n`
	}
	code += `export default ${JSON.stringify(css)}`
	}

Now, given that it doesn't appear to be used anywhere, it may not be necessary for the actual css code to be available in the variable, a simple "null" value would be enough.

Additional context

Fixes #3397.

---

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

[Shinigami92]

Is it also possible to add a test in the playgrounds to prevent regression?

---

Also looks like there are now failing tests 🤔
You can run these also locally with yarn test-build and yarn test-serve

[ferdinando-ferreira]

Also looks like there are now failing tests 🤔
You can run these also locally with yarn test-build and yarn test-serve

Will do and try to understand the reason for the failure

[ferdinando-ferreira]

Also looks like there are now failing tests 🤔
You can run these also locally with yarn test-build and yarn test-serve

Thanks, I was not aware of these. The reason the test was failing was due to a test for an unrelated regression (for #132).

vite/packages/playground/resolve/index.html

Lines 159 to 163 in 30ff5a2

	// css entry
	import css from 'normalize.css'
	if (typeof css === 'string') {
	text('.css', '[success] resolve package with css entry file')
	}

It tested that a css was sucessfully included by verifying if the result was a string.

I changed the returned module from export default null to export default '' because there may be the assumption that importing a css file returns either an object (in case of css modules) or a string (otherwise).

Is it also possible to add a test in the playgrounds to prevent regression?

Yes, working on it.

[ferdinando-ferreira]

@Shinigami92 : done! Took advantage of the existing test for the plugin and added an extra test to ensure it's working properly for imports without variables.

Other than that the existing test proves it is working correctly by showing that the css source code is no longer being returned by the export. Here are the steps, from the root of the project

cd packages/playground/css/
yarn build
yarn serve

There will be a page served from http://localhost:5000/ containing the existing test and the new one, along with the now empty "imported __ string".

That will likely result in a massively reduced build size for entries with imported css across the board. In my project alone, the resulting dist/index.js went from 474K to 14K without change in functionality or behaviour.

$ du -b assets/client/dist/index.js old/client/dist/index.js
 14452  assets/client/dist/index.js
474629  old/client/dist/index.js

Please review and let me know if there is any required change.

[Shinigami92]

Oh no 🙁
Seems there is a failing test

[ferdinando-ferreira]

Seems there is a failing test

There was a typo in the test I added, it must be fixed now.

[thecrypticace]

This breaks production builds that import a css file to get the css text (I use this to style a vue component subtree that's mounted in the shadow DOM). Given that the text is still available in dev it was a bit surprising seeing the behavior difference.

Though, I'm in favor of removing css from the emitted js in prod bundles. It seems like #2148 (comment) could be a possible resolution to this problem.