fix: if the same CSS file is imported in many js files, then the CSS …

…is extracted for the first issuer only, #68
webdiscus · Jan 9, 2024 · f2635a7 · f2635a7
1 parent b7e2f37
commit f2635a7
Show file tree

Hide file tree

Showing 20 changed files with 185 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,12 +1,16 @@
 # Change log
 
+## 3.4.7 (2024-01-09)
+
+- fix: if the same CSS file is imported in many js files, then the CSS is extracted for the first issuer only, #68
+
 ## 3.4.6 (2024-01-02)
 
-fix: the `pathData.chunk.name` is undefined when the `js.filename` is a function, this bug was introduced in `3.4.5`
+- fix: the `pathData.chunk.name` is undefined when the `js.filename` is a function, this bug was introduced in `3.4.5`
 
 ## 3.4.5 (2024-01-01)
 
-fix: the `pathData.filename` is undefined after changes when the `js.filename` is a function
+- fix: the `pathData.filename` is undefined after changes when the `js.filename` is a function
 
 ## 3.4.4 (2023-12-28)
 

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "html-bundler-webpack-plugin",
-  "version": "3.4.6",
+  "version": "3.4.7",
   "description": "HTML bundler plugin for webpack handles a template as an entry point, extracts CSS and JS from their sources referenced in HTML, supports template engines like Eta, EJS, Handlebars, Nunjucks.",
   "keywords": [
     "html",

diff --git a/src/Plugin/AssetCompiler.js b/src/Plugin/AssetCompiler.js
@@ -1001,7 +1001,20 @@ class AssetCompiler {
     const orderedRootIssuers = Collection.orderedResources.get(entry.id);
 
     for (const issuer of orderedRootIssuers) {
-      if (!Collection.importStyleRootIssuers.has(issuer)) continue;
+      // Fix #68: if the same `c.css` file was imported in many js files: `a.js` and `b.js`,
+      // then webpack processes the css module only for 1st `a.js`, others issuers will be ignored,
+      // then we lost relation: a.js -> c.css (ok) but b.js -> c.css (lost).
+      // So we can't use the following check for avoid unnecessary searching in js files where no CSS has been imported
+      // Side-effect: increases build time for cases when many js files do not import css.
+      // TODO: create a cache for js files that don't import CSS.
+      // if (!Collection.importStyleRootIssuers.has(issuer)) {
+      //   console.log('--- importStyleRootIssuers: ', {
+      //     entryFilename,
+      //     issuer,
+      //     importStyleRootIssuers: Collection.importStyleRootIssuers,
+      //   });
+      //   continue;
+      // }
 
       const issuerEntry = AssetEntry.getByResource(issuer);
       const sources = [];

diff --git a/test/cases/js-import-css-nested-deep/expected/css/pageA.5b01e96e.css b/test/cases/js-import-css-nested-deep/expected/css/pageA.5b01e96e.css
@@ -0,0 +1,10 @@
+.bar {
+  color: dodgerblue;
+  background-color: white;
+  border: 1px solid dodgerblue;
+}
+.foo {
+  color: red;
+  background-color: white;
+  border: 1px solid red;
+}
diff --git a/test/cases/js-import-css-nested-deep/expected/css/pageB.b78f128d.css b/test/cases/js-import-css-nested-deep/expected/css/pageB.b78f128d.css
@@ -0,0 +1,10 @@
+.bar {
+  color: dodgerblue;
+  background-color: white;
+  border: 1px solid dodgerblue;
+}
+.foo {
+  color: red;
+  background-color: white;
+  border: 1px solid red;
+}
diff --git a/test/cases/js-import-css-nested-deep/expected/js/pageA.2fcc41eb.js b/test/cases/js-import-css-nested-deep/expected/js/pageA.2fcc41eb.js
@@ -0,0 +1 @@
+(()=>{"use strict";console.log(">> Page A: ",{Foo:"Component Bar",Bar:"Component Foo"})})();
diff --git a/test/cases/js-import-css-nested-deep/expected/js/pageB.af02ad73.js b/test/cases/js-import-css-nested-deep/expected/js/pageB.af02ad73.js
@@ -0,0 +1 @@
+(()=>{"use strict";console.log(">> Page B: ",{Foo:"Component Bar",Bar:"Component Foo"})})();
diff --git a/test/cases/js-import-css-nested-deep/expected/pageA.html b/test/cases/js-import-css-nested-deep/expected/pageA.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Page A</title>
+<link href="css/pageA.5b01e96e.css" rel="stylesheet">
+</head>
+<body>
+  <div style="display: flex; flex-direction: column;">
+    <h1>Page A</h1>
+    <a href="pageB.html">Visit Page B</a>
+    <b>Styled components:</b>
+    <div class="bar">Foo</div>
+    <div class="foo">Bar</div>
+  </div>
+  <script src="js/pageA.2fcc41eb.js"></script>
+</body>
+</html>
diff --git a/test/cases/js-import-css-nested-deep/expected/pageB.html b/test/cases/js-import-css-nested-deep/expected/pageB.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Page B</title>
+<link href="css/pageB.b78f128d.css" rel="stylesheet">
+</head>
+<body>
+  <div style="display: flex; flex-direction: column;">
+    <h1>Page B</h1>
+    <a href="pageA.html">Visit Page A</a>
+    <b>Styled components:</b>
+    <div class="bar">Foo</div>
+    <div class="foo">Bar</div>
+  </div>
+  <script src="js/pageB.af02ad73.js"></script>
+</body>
+</html>
diff --git a/test/cases/js-import-css-nested-deep/src/components/Bar.css b/test/cases/js-import-css-nested-deep/src/components/Bar.css
@@ -0,0 +1,5 @@
+.bar {
+  color: dodgerblue;
+  background-color: white;
+  border: 1px solid dodgerblue;
+}
diff --git a/test/cases/js-import-css-nested-deep/src/components/Bar.js b/test/cases/js-import-css-nested-deep/src/components/Bar.js
@@ -0,0 +1,5 @@
+import './Bar.css';
+
+const name = 'Component Bar';
+
+export default name;
diff --git a/test/cases/js-import-css-nested-deep/src/components/Foo.css b/test/cases/js-import-css-nested-deep/src/components/Foo.css
@@ -0,0 +1,5 @@
+.foo {
+  color: red;
+  background-color: white;
+  border: 1px solid red;
+}
diff --git a/test/cases/js-import-css-nested-deep/src/components/Foo.js b/test/cases/js-import-css-nested-deep/src/components/Foo.js
@@ -0,0 +1,5 @@
+import './Foo.css';
+
+const name = 'Component Foo';
+
+export default name;
diff --git a/test/cases/js-import-css-nested-deep/src/pages/index.css b/test/cases/js-import-css-nested-deep/src/pages/index.css
@@ -0,0 +1,3 @@
+h1 {
+  color: orangered;
+}
diff --git a/test/cases/js-import-css-nested-deep/src/pages/pageA.html b/test/cases/js-import-css-nested-deep/src/pages/pageA.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Page A</title>
+</head>
+<body>
+  <div style="display: flex; flex-direction: column;">
+    <h1>Page A</h1>
+    <a href="pageB.html">Visit Page B</a>
+    <b>Styled components:</b>
+    <div class="bar">Foo</div>
+    <div class="foo">Bar</div>
+  </div>
+  <script src="pageA.js"></script>
+</body>
+</html>
diff --git a/test/cases/js-import-css-nested-deep/src/pages/pageA.js b/test/cases/js-import-css-nested-deep/src/pages/pageA.js
@@ -0,0 +1,4 @@
+import Foo from '../components/Bar';
+import Bar from '../components/Foo';
+
+console.log('>> Page A: ', { Foo, Bar });
diff --git a/test/cases/js-import-css-nested-deep/src/pages/pageB.html b/test/cases/js-import-css-nested-deep/src/pages/pageB.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Page B</title>
+</head>
+<body>
+  <div style="display: flex; flex-direction: column;">
+    <h1>Page B</h1>
+    <a href="pageA.html">Visit Page A</a>
+    <b>Styled components:</b>
+    <div class="bar">Foo</div>
+    <div class="foo">Bar</div>
+  </div>
+  <script src="pageB.js"></script>
+</body>
+</html>
diff --git a/test/cases/js-import-css-nested-deep/src/pages/pageB.js b/test/cases/js-import-css-nested-deep/src/pages/pageB.js
@@ -0,0 +1,4 @@
+import Foo from '../components/Bar';
+import Bar from '../components/Foo';
+
+console.log('>> Page B: ', { Foo, Bar });
diff --git a/test/cases/js-import-css-nested-deep/webpack.config.js b/test/cases/js-import-css-nested-deep/webpack.config.js
@@ -0,0 +1,44 @@
+const path = require('path');
+const HtmlBundlerPlugin = require('@test/html-bundler-webpack-plugin');
+
+// test: fix of the issue #68
+
+module.exports = {
+  mode: 'production',
+  //mode: 'development',
+
+  output: {
+    path: path.join(__dirname, 'dist/'),
+  },
+
+  plugins: [
+    new HtmlBundlerPlugin({
+      //entry: 'src/',
+      entry: {
+        pageA: './src/pages/pageA.html',
+        pageB: './src/pages/pageB.html',
+      },
+
+      js: {
+        filename: 'js/[name].[contenthash:8].js',
+      },
+
+      css: {
+        filename: 'css/[name].[contenthash:8].css',
+      },
+    }),
+  ],
+
+  module: {
+    rules: [
+      {
+        test: /\.(css)$/,
+        use: ['css-loader'],
+      },
+    ],
+  },
+
+  optimization: {
+    usedExports: true,
+  },
+};
diff --git a/test/integration.test.js b/test/integration.test.js
@@ -279,6 +279,7 @@ describe('import styles in JavaScript', () => {
   // special cases
   test('nested import, sorted', () => compareFiles('js-import-css-nested-sorted'));
   test('order dependencies', () => compareFiles('js-import-css-order-dependencies'));
+  test('import the same css in deep in  many js', () => compareFiles('js-import-css-nested-deep'));
 
   test('import one CSS from many JS files', () => compareFiles('js-import-css-one-from-many-js'));
   test('multiple-pages-same-asset', () => compareFiles('js-import-css-multiple-pages-same-asset'));