Repository: tivac/modular-css
Pull request #857 (2022-06-15 19:23:41)

fix: only include direct deps in compositions 
<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->
Only compose from direct dependencies, not the entire chain

## 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. -->
Because of how the `@modular-css/css-in-js` package works the deps already contain all of *their* deps, so by concatenating all of the deps of the selector you actually end up with duplicated classes in the output and it looks a real mess.

## 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 a test & snapshot

## 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.


codesee-maps[bot]: <details>
<summary><b>👇 <a href="https://app.codesee.io/r/reviews?pr=857&src=https%3A%2F%2Fgithub.com%2Ftivac%2Fmodular-css">Click on the image</a> for a new way to code review</b>
</summary>

  - Make big changes easier — review code in small groups of related files
  - Know where to start — see the whole change at a glance
  - Take a code tour — explore the change with an interactive tour
  - Make comments and review — all fully sync’ed with github

    [Try it now!](https://app.codesee.io/r/reviews?pr=857&src=https%3A%2F%2Fgithub.com%2Ftivac%2Fmodular-css)
</details>

[![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/tivac/modular-css/857/d24399c3/086703b2f69872dc8776370339475adc50ff45d8.svg)](https://app.codesee.io/r/reviews?pr=857&src=https%3A%2F%2Fgithub.com%2Ftivac%2Fmodular-css)

#### Legend
![CodeSee Map Legend](https://app.codesee.io/Legend_v2.png)
netlify[bot]: ### <span aria-hidden="true">✅</span> Deploy Preview for *m-css* ready!


|  Name | Link |
|---------------------------------|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 086703b2f69872dc8776370339475adc50ff45d8 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/m-css/deploys/62aa31bfd30c6a000932f110 |
|<span aria-hidden="true">😎</span> Deploy Preview | https://deploy-preview-857--m-css.netlify.app |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTg1Ny0tbS1jc3MubmV0bGlmeS5hcHAifQ.L4lwBS-kPyIvhqJ8BqbKIIU50V07FKApdx-omMJxo6M)<br /><br />_Use your smartphone camera to open QR code link._</details> |
---

_To edit notification comments on pull requests, go to your [Netlify site settings](https://app.netlify.com/sites/m-css/settings/deploys#deploy-notifications)._
codecov[bot]: # [Codecov](https://codecov.io/gh/tivac/modular-css/pull/857?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) Report
> Merging [#857](https://codecov.io/gh/tivac/modular-css/pull/857?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) (086703b) into [main](https://codecov.io/gh/tivac/modular-css/commit/8dc538bffc4217d5c532f7b8f74ee2bba2ce8cd6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) (8dc538b) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #857      +/-   ##
==========================================
- Coverage   99.58%   99.58%   -0.01%     
==========================================
  Files          37       37              
  Lines        1437     1436       -1     
  Branches      231      231              
==========================================
- Hits         1431     1430       -1     
  Misses          6        6              
```


| [Impacted Files](https://codecov.io/gh/tivac/modular-css/pull/857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) | Coverage Δ | |
|---|---|---|
| [packages/css-to-js/css-to-js.js](https://codecov.io/gh/tivac/modular-css/pull/857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit#diff-cGFja2FnZXMvY3NzLXRvLWpzL2Nzcy10by1qcy5qcw==) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tivac/modular-css/pull/857?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/tivac/modular-css/pull/857?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit). Last update [8dc538b...086703b](https://codecov.io/gh/tivac/modular-css/pull/857?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit).


Patch:
diff --git a/packages/css-to-js/css-to-js.js b/packages/css-to-js/css-to-js.js
index 766b4a58..385037a3 100644
--- a/packages/css-to-js/css-to-js.js
+++ b/packages/css-to-js/css-to-js.js
@@ -68,10 +68,10 @@ exports.transform = (file, processor, opts = {}) => {
 
     // Only want direct dependencies and any first-level dependencies
     // of this file to be processed
-    graph.outgoingEdges[Processor.fileKey(id)].forEach((dep) => {
+    graph.directDependenciesOf(Processor.fileKey(id)).forEach((dep) => {
         dependencies.add(dep);
 
-        graph.outgoingEdges[dep].forEach((d) => {
+        graph.directDependenciesOf(dep).forEach((d) => {
             dependencies.add(d);
         });
     });
@@ -215,15 +215,14 @@ exports.transform = (file, processor, opts = {}) => {
 
         // Build the list of composed classes for this class
         if(graph.hasNode(sKey)) {
-            graph.dependenciesOf(sKey).forEach((dep) => {
+            graph.directDependenciesOf(sKey).forEach((dep) => {
                 const { file: src, selector } = graph.getNodeData(dep);
 
                 // Get the value from the right place
-                if(src !== id) {
-                    elements.push(externalsMap.get(dep));
-                } else {
-                    elements.push(internalsMap.get(selector));
-                }
+                elements.push(src !== id ?
+                    externalsMap.get(dep) :
+                    internalsMap.get(selector)
+                );
             });
         }
 


Test patch:
diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap
index 2d3f5d65..ec18a9ae 100644
--- a/packages/rollup/test/__snapshots__/rollup.test.js.snap
+++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap
@@ -268,21 +268,51 @@ Object {
 }
 `;
 
+exports[`/rollup.js should express layers of locally-composed classes correctly 1`] = `
+Object {
+  "assets/composition-layers.css": "
+/* packages/rollup/test/specimens/composition-layers/composition-layers.css */
+.mc_a {
+    color: red;
+}
+
+.mc_b {
+    color: blue;
+}
+
+.mc_c {
+    color: green;
+}",
+  "composition-layers.js": "
+const a = \\"mc_a\\";
+const b = a + \\" \\" + \\"mc_b\\";
+const c = b + \\" \\" + \\"mc_c\\";
+var css = {
+    a,
+b,
+c
+};
+
+console.log(css);
+",
+}
+`;
+
 exports[`/rollup.js should express local & remote-composed classes correctly 1`] = `
 Object {
-  "assets/multiple-composition.css": "
-/* packages/rollup/test/specimens/multiple-composition/other.css */
+  "assets/internal-external-composition.css": "
+/* packages/rollup/test/specimens/internal-external-composition/other.css */
 .mc_other {
     color: green;
 }
-/* packages/rollup/test/specimens/multiple-composition/multiple-composition.css */
+/* packages/rollup/test/specimens/internal-external-composition/internal-external-composition.css */
 .mc_one {
     color: red;
 }
 .mc_two {
     background: blue;
 }",
-  "multiple-composition.js": "
+  "internal-external-composition.js": "
 const other = \\"mc_other\\";
 
 const one = \\"mc_one\\";
diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js
index 6a2a890c..38d3eef3 100644
--- a/packages/rollup/test/rollup.test.js
+++ b/packages/rollup/test/rollup.test.js
@@ -72,9 +72,25 @@ describe("/rollup.js", () => {
         ).toMatchRollupSnapshot();
     });
 
+    it("should express layers of locally-composed classes correctly", async () => {
+        const bundle = await rollup({
+            input   : require.resolve(`./specimens/composition-layers/composition-layers.js`),
+            plugins : [
+                createPlugin(),
+            ],
+        });
+
+        expect(
+            await bundle.generate({
+                format,
+                assetFileNames,
+            })
+        ).toMatchRollupSnapshot();
+    });
+
     it("should express local & remote-composed classes correctly", async () => {
         const bundle = await rollup({
-            input   : require.resolve(`./specimens/multiple-composition/multiple-composition.js`),
+            input   : require.resolve(`./specimens/internal-external-composition/internal-external-composition.js`),
             plugins : [
                 createPlugin(),
             ],
diff --git a/packages/rollup/test/specimens/composition-layers/composition-layers.css b/packages/rollup/test/specimens/composition-layers/composition-layers.css
new file mode 100644
index 00000000..7289d1b8
--- /dev/null
+++ b/packages/rollup/test/specimens/composition-layers/composition-layers.css
@@ -0,0 +1,15 @@
+.a {
+    color: red;
+}
+
+.b {
+    composes: a;
+
+    color: blue;
+}
+
+.c {
+    composes: b;
+
+    color: green;
+}
\ No newline at end of file
diff --git a/packages/rollup/test/specimens/composition-layers/composition-layers.js b/packages/rollup/test/specimens/composition-layers/composition-layers.js
new file mode 100644
index 00000000..cae34e3b
--- /dev/null
+++ b/packages/rollup/test/specimens/composition-layers/composition-layers.js
@@ -0,0 +1,3 @@
+import css from "./composition-layers.css";
+
+console.log(css);
\ No newline at end of file
diff --git a/packages/rollup/test/specimens/multiple-composition/multiple-composition.css b/packages/rollup/test/specimens/internal-external-composition/internal-external-composition.css
similarity index 100%
rename from packages/rollup/test/specimens/multiple-composition/multiple-composition.css
rename to packages/rollup/test/specimens/internal-external-composition/internal-external-composition.css
diff --git a/packages/rollup/test/specimens/internal-external-composition/internal-external-composition.js b/packages/rollup/test/specimens/internal-external-composition/internal-external-composition.js
new file mode 100644
index 00000000..7cb5970d
--- /dev/null
+++ b/packages/rollup/test/specimens/internal-external-composition/internal-external-composition.js
@@ -0,0 +1,3 @@
+import css from "./internal-external-composition.css";
+
+console.log(css);
diff --git a/packages/rollup/test/specimens/multiple-composition/other.css b/packages/rollup/test/specimens/internal-external-composition/other.css
similarity index 100%
rename from packages/rollup/test/specimens/multiple-composition/other.css
rename to packages/rollup/test/specimens/internal-external-composition/other.css
diff --git a/packages/rollup/test/specimens/multiple-composition/multiple-composition.js b/packages/rollup/test/specimens/multiple-composition/multiple-composition.js
deleted file mode 100644
index 2f89867d..00000000
--- a/packages/rollup/test/specimens/multiple-composition/multiple-composition.js
+++ /dev/null
@@ -1,3 +0,0 @@
-import css from "./multiple-composition.css";
-
-console.log(css);