Repository: tivac/modular-css
Pull request #748 (2020-04-21 19:24:13)

fix(rollup): walk full tree for dependencies
<!--- Provide a general summary of your changes in the Title above -->

## Description
Using `this.getModuleInfo()` plugin hook to get better information about the module graph so it can be fully traversed, to ensure that all CSS are properly represented and can be chunked correctly.

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Tired of CSS missing from complicated chunking setups

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
Added tests, also verified against work codebase which is *very* complex and suffered from this bug.

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the **CONTRIBUTING** document.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.



Patch:
diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js
index fd2b428a..5a0f3e61 100644
--- a/packages/rollup/rollup.js
+++ b/packages/rollup/rollup.js
@@ -195,11 +195,34 @@ module.exports = (opts = {}) => {
                 if(type === "asset") {
                     return;
                 }
-                
-                // Get CSS files being used by this chunk
-                const css = Object.keys(modules).filter((file) => processor.has(file));
 
-                if(!css.length) {
+                // Get CSS files being used by this chunk or any of its dependencies
+                const queue = Object.keys(modules);
+                const seen = new Set();
+                const deps = [];
+
+                while(queue.length) {
+                    const module = queue.shift();
+
+                    seen.add(module);
+                    
+                    // Only care about CSS dependencies for this
+                    if(processor.has(module)) {
+                        deps.push(module);
+                    }
+
+                    const { importedIds } = this.getModuleInfo(module);
+
+                    importedIds.forEach((dep) => {
+                        if(seen.has(dep)) {
+                            return;
+                        }
+
+                        queue.push(dep);
+                    });
+                }
+
+                if(!deps.length) {
                     return;
                 }
 
@@ -210,7 +233,7 @@ module.exports = (opts = {}) => {
                 // @modular-css/svelte is broken atm because this just splats over the top of it
                 graph.addNode(entry, [ entry ]);
 
-                css.forEach((file) => graph.addDependency(entry, processor.normalize(file)));
+                deps.forEach((file) => graph.addDependency(entry, processor.normalize(file)));
             });
 
             // Output CSS chunks


Test patch:
diff --git a/packages/rollup/test/__snapshots__/splitting.test.js.snap b/packages/rollup/test/__snapshots__/splitting.test.js.snap
index 945fa7ba..00a3ecd9 100644
--- a/packages/rollup/test/__snapshots__/splitting.test.js.snap
+++ b/packages/rollup/test/__snapshots__/splitting.test.js.snap
@@ -337,16 +337,14 @@ Object {
 }
 ",
   "assets/c.css": "
-/* packages/rollup/test/specimens/css-chunks/c.css */
-.c {
-    color: cyan;
-}
-",
-  "assets/shared.css": "
 /* packages/rollup/test/specimens/css-chunks/shared.css */
 .shared {
     color: snow;
 }
+/* packages/rollup/test/specimens/css-chunks/c.css */
+.c {
+    color: cyan;
+}
 ",
 }
 `;
diff --git a/packages/rollup/test/__snapshots__/watch.test.js.snap b/packages/rollup/test/__snapshots__/watch.test.js.snap
index 107d03fb..33cda223 100644
--- a/packages/rollup/test/__snapshots__/watch.test.js.snap
+++ b/packages/rollup/test/__snapshots__/watch.test.js.snap
@@ -147,14 +147,16 @@ Snapshot Diff:
 + Second value
 
 @@ --- --- @@
-  Object {
-    "assets/one.css": "/* packages/rollup/test/output/watch/code-splitting/shared.css */
+  .mc204ad279_one {
+      color: red;
+  }",
+    "assets/shared.css": "/* packages/rollup/test/output/watch/code-splitting/shared.css */
   .mc4002e81f_shared {
 -     color: blue;
 +     color: seafoam;
-  }
-  /* packages/rollup/test/output/watch/code-splitting/one.css */
-  .mc204ad279_one {
-      color: red;
+  }",
+    "assets/two.css": "/* packages/rollup/test/output/watch/code-splitting/two.css */
+  .mc3861d3af_two {
+      color: green;
   }",
 `;
diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js
index 8d641fde..aa748642 100644
--- a/packages/rollup/test/rollup.test.js
+++ b/packages/rollup/test/rollup.test.js
@@ -646,7 +646,7 @@ describe("/rollup.js", () => {
         let fn = it;
 
         // Verify that filesystem is case-insensitive before bothering
-        fs.writeFileSync("./packages/rollup/test/output/sensitive.txt");
+        fs.writeFileSync("./packages/rollup/test/output/sensitive.txt", "");
 
         try {
             fs.statSync("./packages/rollup/test/output/SENSITIVE.txt");