Repository: tivac/modular-css
Pull request #856 (2022-06-15 06:57:25)

fix: don't move composed classes early
Because it can cause invalid output if they get moved ahead of their dependencies, and we don't need to handle them at this point anyways.

codesee-maps[bot]: <details>
<summary><b>👇 <a href="https://app.codesee.io/r/reviews?pr=856&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=856&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/856/8d39f227/74494510c9b69eb663459977176188e206ae1098.svg)](https://app.codesee.io/r/reviews?pr=856&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 | 74494510c9b69eb663459977176188e206ae1098 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/m-css/deploys/62a982d78c84b800095cf946 |
|<span aria-hidden="true">😎</span> Deploy Preview | https://deploy-preview-856--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.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTg1Ni0tbS1jc3MubmV0bGlmeS5hcHAifQ.nBjy2F6v2QghMc5Md_AtGScSV5UVLovCjsVT8hlUFv4)<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/856?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) Report
> Merging [#856](https://codecov.io/gh/tivac/modular-css/pull/856?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) (7449451) into [main](https://codecov.io/gh/tivac/modular-css/commit/dc476efbc871693beb975039593f6d574eb0fd42?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) (dc476ef) will **decrease** coverage by `0.00%`.
> The diff coverage is `n/a`.

> :exclamation: Current head 7449451 differs from pull request most recent head c1556fb. Consider uploading reports for the commit c1556fb to get more accurate results

```diff
@@            Coverage Diff             @@
##             main     #856      +/-   ##
==========================================
- Coverage   99.58%   99.58%   -0.01%     
==========================================
  Files          37       37              
  Lines        1439     1437       -2     
  Branches      232      231       -1     
==========================================
- Hits         1433     1431       -2     
  Misses          6        6              
```


| [Impacted Files](https://codecov.io/gh/tivac/modular-css/pull/856?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/856/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% <ø> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/tivac/modular-css/pull/856?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/856?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit). Last update [dc476ef...c1556fb](https://codecov.io/gh/tivac/modular-css/pull/856?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 51067524..766b4a58 100644
--- a/packages/css-to-js/css-to-js.js
+++ b/packages/css-to-js/css-to-js.js
@@ -26,7 +26,6 @@ const DEFAULTS = {
 const {
     selectorKey,
     isFile,
-    isSelector,
     isValue,
 } = Processor;
 
@@ -108,14 +107,8 @@ exports.transform = (file, processor, opts = {}) => {
 
         const imported = importsMap.get(depFile);
 
-        // File we're transforming
+        // Local deps are ignored at this point
         if(depFile === id) {
-            // Track this selector as part of the keys to be exported, adding
-            // here so it'll be sorted topologically
-            if(isSelector(depKey)) {
-                exportedKeys.add(data.selector);
-            }
-
             return;
         }
 


Test patch:
diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap
index 64971e85..2d3f5d65 100644
--- a/packages/rollup/test/__snapshots__/rollup.test.js.snap
+++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap
@@ -268,9 +268,38 @@ Object {
 }
 `;
 
+exports[`/rollup.js should express local & remote-composed classes correctly 1`] = `
+Object {
+  "assets/multiple-composition.css": "
+/* packages/rollup/test/specimens/multiple-composition/other.css */
+.mc_other {
+    color: green;
+}
+/* packages/rollup/test/specimens/multiple-composition/multiple-composition.css */
+.mc_one {
+    color: red;
+}
+.mc_two {
+    background: blue;
+}",
+  "multiple-composition.js": "
+const other = \\"mc_other\\";
+
+const one = \\"mc_one\\";
+const two = other + \\" \\" + one + \\" \\" + \\"mc_two\\";
+var css = {
+    one,
+two
+};
+
+console.log(css);
+",
+}
+`;
+
 exports[`/rollup.js should express locally-composed classes correctly 1`] = `
 Object {
-  "assets/local-composition-1485bb4a.css": "
+  "assets/local-composition.css": "
 /* packages/rollup/test/specimens/local-composition.css */
 .mc_one {
     color: red;
diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js
index 2db79654..6a2a890c 100644
--- a/packages/rollup/test/rollup.test.js
+++ b/packages/rollup/test/rollup.test.js
@@ -51,6 +51,7 @@ describe("/rollup.js", () => {
         expect(
             await bundle.generate({
                 format,
+                assetFileNames,
             })
         ).toMatchRollupCodeSnapshot();
     });
@@ -66,6 +67,23 @@ describe("/rollup.js", () => {
         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`),
+            plugins : [
+                createPlugin(),
+            ],
+        });
+
+        expect(
+            await bundle.generate({
+                format,
+                assetFileNames,
             })
         ).toMatchRollupSnapshot();
     });
@@ -81,6 +99,7 @@ describe("/rollup.js", () => {
         expect(
             await bundle.generate({
                 format,
+                assetFileNames,
             })
         ).toMatchRollupCodeSnapshot();
     });
diff --git a/packages/rollup/test/specimens/multiple-composition/multiple-composition.css b/packages/rollup/test/specimens/multiple-composition/multiple-composition.css
new file mode 100644
index 00000000..23734953
--- /dev/null
+++ b/packages/rollup/test/specimens/multiple-composition/multiple-composition.css
@@ -0,0 +1,10 @@
+.one {
+    color: red;
+}
+
+.two {
+    composes: other from "./other.css";
+    composes: one;
+
+    background: blue;
+}
diff --git a/packages/rollup/test/specimens/multiple-composition/multiple-composition.js b/packages/rollup/test/specimens/multiple-composition/multiple-composition.js
new file mode 100644
index 00000000..2f89867d
--- /dev/null
+++ b/packages/rollup/test/specimens/multiple-composition/multiple-composition.js
@@ -0,0 +1,3 @@
+import css from "./multiple-composition.css";
+
+console.log(css);
diff --git a/packages/rollup/test/specimens/multiple-composition/other.css b/packages/rollup/test/specimens/multiple-composition/other.css
new file mode 100644
index 00000000..15b74b04
--- /dev/null
+++ b/packages/rollup/test/specimens/multiple-composition/other.css
@@ -0,0 +1,3 @@
+.other {
+    color: green;
+}
\ No newline at end of file