From 30a2e15ecb04b5ec26d422f838757f2d92e779bb Mon Sep 17 00:00:00 2001 From: Ryan Christian Date: Mon, 14 Sep 2020 14:25:51 -0500 Subject: [PATCH 1/7] fix: Enabling sideEffects for style loaders --- packages/cli/lib/lib/webpack/webpack-base-config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/cli/lib/lib/webpack/webpack-base-config.js b/packages/cli/lib/lib/webpack/webpack-base-config.js index 1b6923ffb..0dcf3bfe8 100644 --- a/packages/cli/lib/lib/webpack/webpack-base-config.js +++ b/packages/cli/lib/lib/webpack/webpack-base-config.js @@ -247,6 +247,7 @@ module.exports = function (env) { }, }, ], + sideEffects: true, }, { // External / `node_module` styles @@ -269,6 +270,7 @@ module.exports = function (env) { }, }, ], + sideEffects: true, }, { test: /\.(xml|html|txt|md)$/, From 7d80cb64a0621b4564a17eeb4a1f4b72aa74851b Mon Sep 17 00:00:00 2001 From: Ryan Christian Date: Mon, 14 Sep 2020 22:44:45 -0500 Subject: [PATCH 2/7] refactor: Don't need to enable sideEffects with modules --- packages/cli/lib/lib/webpack/webpack-base-config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/lib/lib/webpack/webpack-base-config.js b/packages/cli/lib/lib/webpack/webpack-base-config.js index 0dcf3bfe8..01185cfd0 100644 --- a/packages/cli/lib/lib/webpack/webpack-base-config.js +++ b/packages/cli/lib/lib/webpack/webpack-base-config.js @@ -247,7 +247,6 @@ module.exports = function (env) { }, }, ], - sideEffects: true, }, { // External / `node_module` styles From 1097cf321588d637e3f62f5724699b4bd307549d Mon Sep 17 00:00:00 2001 From: Ryan Christian Date: Tue, 15 Sep 2020 00:09:10 -0500 Subject: [PATCH 3/7] chore(ignore): Excluding new `size-plugin-ssr` --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 59c9840d2..4fcd6b5c8 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ tmp-*.json # auto generated while running tests packages/cli/size-plugin.json +packages/cli/size-plugin-ssr.json From a9eb285df30d741c6629eddbaa52ff101e5ad1b2 Mon Sep 17 00:00:00 2001 From: Ryan Christian Date: Tue, 15 Sep 2020 00:10:04 -0500 Subject: [PATCH 4/7] test: Adding test to ensure side effects ignored for non-modules css imports --- packages/cli/tests/build.test.js | 10 ++++++++++ packages/cli/tests/images/build.js | 17 +++++++++++++++++ .../cli/tests/subjects/side-effect-css/index.js | 6 ++++++ .../tests/subjects/side-effect-css/package.json | 5 +++++ .../tests/subjects/side-effect-css/style.css | 3 +++ 5 files changed, 41 insertions(+) create mode 100644 packages/cli/tests/subjects/side-effect-css/index.js create mode 100644 packages/cli/tests/subjects/side-effect-css/package.json create mode 100644 packages/cli/tests/subjects/side-effect-css/style.css diff --git a/packages/cli/tests/build.test.js b/packages/cli/tests/build.test.js index 65b4ddb12..ee30b2f79 100644 --- 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 index 0d6f311f7..16affa9a1 100644 --- a/packages/cli/tests/images/build.js +++ b/packages/cli/tests/images/build.js @@ -55,6 +55,23 @@ exports.sass = ` `; +exports.sideEffectCss = ` + + + 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 new file mode 100644 index 000000000..fd48dbdf4 --- /dev/null +++ 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 new file mode 100644 index 000000000..54dee66f7 --- /dev/null +++ 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 new file mode 100644 index 000000000..d13e56c0f --- /dev/null +++ b/packages/cli/tests/subjects/side-effect-css/style.css @@ -0,0 +1,3 @@ +h1 { + background: #673ab8; +} From bd8816814157daa9568c494761bd1c522e8846a1 Mon Sep 17 00:00:00 2001 From: Ryan Christian <33403762+RyanChristian4427@users.noreply.github.com> Date: Tue, 15 Sep 2020 13:10:10 -0500 Subject: [PATCH 5/7] refactor: Adding comment to explain --- packages/cli/lib/lib/webpack/webpack-base-config.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/lib/lib/webpack/webpack-base-config.js b/packages/cli/lib/lib/webpack/webpack-base-config.js index 01185cfd0..4cda50a71 100644 --- a/packages/cli/lib/lib/webpack/webpack-base-config.js +++ b/packages/cli/lib/lib/webpack/webpack-base-config.js @@ -269,6 +269,10 @@ 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, }, { From 9ecd194e2147eb200bd4c0556bdf0a21aae310b9 Mon Sep 17 00:00:00 2001 From: Ryan Christian <ryanchristian4427@gmail.com> Date: Wed, 23 Sep 2020 00:22:56 -0500 Subject: [PATCH 6/7] fix(changeset): Adding changeset --- .changeset/gold-pumas-invite.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gold-pumas-invite.md diff --git a/.changeset/gold-pumas-invite.md b/.changeset/gold-pumas-invite.md new file mode 100644 index 000000000..9aa030568 --- /dev/null +++ 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. From 4dc6675a62edfba7c94224f2803b971a6e86174a Mon Sep 17 00:00:00 2001 From: Ryan Christian <33403762+rschristian@users.noreply.github.com> Date: Mon, 21 Sep 2020 22:15:53 -0500 Subject: [PATCH 7/7] refactor: Removing `size-plugin-ssr.json` from gitignore --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 4fcd6b5c8..59c9840d2 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,3 @@ tmp-*.json # auto generated while running tests packages/cli/size-plugin.json -packages/cli/size-plugin-ssr.json