fix: Enabling sideEffects on style loaders (#1423)

Co-authored-by: Ryan Christian <[email protected]>
preactjs · Mar 9, 2021 · cc2f3e7 · cc2f3e7
1 parent c33f020
commit cc2f3e7
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 0 deletions.
diff --git a/.changeset/gold-pumas-invite.md b/.changeset/gold-pumas-invite.md
@@ -0,0 +1,5 @@
+---
+'preact-cli': patch
+---
+
+Fixes bug with style loader that would strip non-module CSS files if 'sideEffects' was set to false for the package.
diff --git a/packages/cli/lib/lib/webpack/webpack-base-config.js b/packages/cli/lib/lib/webpack/webpack-base-config.js
@@ -269,6 +269,11 @@ module.exports = function (env) {
 							},
 						},
 					],
+					// Don't consider CSS imports dead code even if the
+					// containing package claims to have no side effects.
+					// Remove this when webpack adds a warning or an error for this.
+					// See https://github.com/webpack/webpack/issues/6571
+					sideEffects: true,
 				},
 				{
 					test: /\.(xml|html|txt|md)$/,

diff --git a/packages/cli/tests/build.test.js b/packages/cli/tests/build.test.js
@@ -208,6 +208,16 @@ describe('preact build', () => {
 		expect(() => build(dir)).not.toThrow();
 	});
 
+	it('should import non-modules CSS even when side effects are false', async () => {
+		let dir = await subject('side-effect-css');
+		await build(dir);
+
+		let head = await getHead(dir);
+		expect(head).toEqual(
+			expect.stringMatching(getRegExpFromMarkup(images.sideEffectCss))
+		);
+	});
+
 	it('should copy resources from static to build directory', async () => {
 		let dir = await subject('static-root');
 		await build(dir);

diff --git a/packages/cli/tests/images/build.js b/packages/cli/tests/images/build.js
@@ -53,6 +53,23 @@ exports.sass = `
 </body>
 `;
 
+exports.sideEffectCss = `
+<head>
+  <meta charset="utf-8">
+  <title>side-effect-css<\\/title>
+  <meta name="viewport" content="width=device-width,initial-scale=1">
+  <meta name="mobile-web-app-capable" content="yes">
+  <meta name="apple-mobile-web-app-capable" content="yes">
+  <link rel="apple-touch-icon" href=\\"\\/assets\\/icons\\/apple-touch-icon\\.png\\">
+  <link rel="manifest" href="\\/manifest\\.json">
+  <style>h1{background:#673ab8}<\\/style>
+  <link href=\\"/bundle.\\w{5}.css\\" rel=\\"stylesheet\\" media=\\"only x\\" onload=\\"this.media='all'\\">
+	<noscript>
+    <link rel=\\"stylesheet\\" href=\\"\\/bundle.\\w{5}.css\\">
+  </noscript>
+<\\/head>
+`;
+
 exports.prerender = {};
 
 exports.prerender.heads = {};

diff --git a/packages/cli/tests/subjects/side-effect-css/index.js b/packages/cli/tests/subjects/side-effect-css/index.js
@@ -0,0 +1,6 @@
+import { h } from 'preact';
+import './style.css';
+
+export default () => {
+	return <h1>SideEffect CSS test</h1>;
+};
diff --git a/packages/cli/tests/subjects/side-effect-css/package.json b/packages/cli/tests/subjects/side-effect-css/package.json
@@ -0,0 +1,5 @@
+{
+  "private": true,
+  "name": "side-effect-css",
+  "sideEffects": false
+}
diff --git a/packages/cli/tests/subjects/side-effect-css/style.css b/packages/cli/tests/subjects/side-effect-css/style.css
@@ -0,0 +1,3 @@
+h1 {
+	background: #673ab8;
+}