Repository: tivac/modular-css
Pull request #1018 (2025-01-07 22:34:14)

Handle global composition
<!--- Provide a general summary of your changes in the Title above -->

## Description
Global composition seems to have broken after version 24 of @modular-css/rollup. What we used to be able to do is have a class defined as:

```css
.a {
  composes: global(foo), global(goo), global(boo);
  color: red;
}
```

that would resolve in markup to `"foo goo boo mc{hash}"`

However, when attempting to do so now, we get `000mc{hash}` where every 0 represents a global style that was lost. 

The culprit seems to lie in this code block (in css-to-js package):

```js
if(graph.hasNode(sKey)) {
            graph.directDependenciesOf(sKey).forEach((dep) => {
                const { file: src, selector } = graph.getNodeData(dep);

                // Get the value from the right place
                elements.push(src !== id ?
                    externalsMap.get(dep) :
                    internalsMap.get(selector)
                );
            });
        }

        elements.push(...details.classes[key].map((t) => JSON.stringify(t)));

        out.push(`${variableDeclaration} ${unique} = ${elements.join(` + " " + `)};`);
```

where the global styles were not found in the `externalsMap`, causing `elements` to become `[ undefined, undefined, undefined, "mc{hash}" ]` and then becoming the output `0 + 0 + 0 + mc{hash}`. 

## Motivation and Context
This bug prevents us from updating m-css and rollup with our current application which uses an external stylesheet separate from the codebase.

## How Has This Been Tested?
I tested this in the relevant application environment and got the styles to compose correctly and wrote a test for this specific use-case. I don't think this change is complete however, because I get some console noise such as this:

```bash
"pureU1", "pureUSm524", "pureMenu", "pureMenuHorizontal", "serifCaps", "pureUSm1924", "pureMenuScrollable", "pureForm", "pureButton", "pureButtonPrimary", "pureUMd13", "pureMenuLink", "pureUSm1324", "serif", "pureUSm1624", "pureUMd1124", "pureG", "pureUMd1924", "pureUSm34", "pureButtonDisabled", "pureU", "pureUSm1424", "table", "module" and "pureU13" are imported from external module "global" but never used in ...
```

I also think the quotations I needed to add around the unchanged global classname to avoid client errors where "global" was an undefined variable seems really strange and will be confusing for future dev work and is probably me not understanding what all is happening here.  Pasting for clarity: 

```js
if(depFile === "global") {
            const unique = deconflict(identifiers, data.selector);

            // wrap un-rewritten selector in quotes so it isn't exported as a variable
            externalsMap.set(selectorKey(depFile, data.selector), rewriteInvalid ?
                unique :
                `"${data.selector}"`
            );

            return;
}
```



## 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! -->
- [ ] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the **CONTRIBUTING** document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
- [ ] I have added a changeset for my change.


changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 7725445a4224b80b244be4f633f8d81bcb9566a1

**The changes in this PR will be included in the next version bump.**

<details><summary>This PR includes changesets to release 1 package</summary>

  | Name                 | Type  |
| -------------------- | ----- |
| @modular-css/webpack | Patch |

</details>

Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/hjhopp/modular-css/new/feat/handle-globals?filename=.changeset/tiny-crabs-cough.md&value=---%0A%22%40modular-css%2Fcss-to-js%22%3A%20patch%0A---%0A%0AHandle%20global%20composition%0A)


netlify[bot]: ### <span aria-hidden="true">✅</span> Deploy Preview for *m-css* ready!


|  Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 7725445a4224b80b244be4f633f8d81bcb9566a1 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/m-css/deploys/677dabe9bd895700083806cb |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-1018--m-css.netlify.app](https://deploy-preview-1018--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.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTEwMTgtLW0tY3NzLm5ldGxpZnkuYXBwIn0.8JJR_jbvhYUjJ8lMExYReVS95LpniHaVp87lY2tRoRE)<br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [m-css Preview](https://deploy-preview-1018--m-css.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify site configuration](https://app.netlify.com/sites/m-css/configuration/notifications#deploy-webhooks)._

Patch:
diff --git a/.changeset/empty-tools-happen.md b/.changeset/empty-tools-happen.md
new file mode 100644
index 00000000..80e833c6
--- /dev/null
+++ b/.changeset/empty-tools-happen.md
@@ -0,0 +1,5 @@
+---
+"@modular-css/webpack": patch
+---
+
+Join class compositions for JSON output
diff --git a/packages/css-to-js/css-to-js.js b/packages/css-to-js/css-to-js.js
index f34f2aa4..9d39dcb8 100644
--- a/packages/css-to-js/css-to-js.js
+++ b/packages/css-to-js/css-to-js.js
@@ -76,24 +76,24 @@ exports.transform = (file, processor, opts = {}) => {
 
     const warnings = [];
     const dependencies = new Set();
-    
+
     // All used identifiers
     const identifiers = new Map();
-    
+
     // External identifiers mapped to their unique names
     const externalsMap = new Map();
-    
+
     // Internal identifiers mapped to their unique names
     const internalsMap = new Map();
-    
+
     // Map of files & their imports
     const importsMap = new Map();
-    
+
     const out = [];
     const defaultExports = [];
     const namedExports = [];
     const valueExports = new Map();
-    
+
     // All the class keys exported by this module
     const exportedKeys = new Set();
 
@@ -108,7 +108,7 @@ exports.transform = (file, processor, opts = {}) => {
             warnings,
         };
     }
-    
+
     // Only want direct dependencies and any first-level dependencies
     // of this file to be processed
     graph.directDependenciesOf(Processor.fileKey(id)).forEach((dep) => {
@@ -120,6 +120,7 @@ exports.transform = (file, processor, opts = {}) => {
     });
 
     // create import statements for all of the values used in compositions
+    /* eslint-disable-next-line max-statements -- this function is long */
     dependencies.forEach((depKey) => {
         const data = graph.getNodeData(depKey);
         const { file : depFile } = data;
@@ -181,6 +182,13 @@ exports.transform = (file, processor, opts = {}) => {
 
             return;
         }
+
+        if(data.global) {
+            // wrap selector in quotes so it isn't exported as a variable
+            externalsMap.set(selectorKey(depFile, data.selector), `"${data.selector}"`);
+
+            return;
+        }
     });
 
     // Write out all the imports
@@ -190,7 +198,7 @@ exports.transform = (file, processor, opts = {}) => {
         }
 
         const source = options.relativeImports ? `./${path.relative(path.dirname(file), from)}` : from;
-        
+
         const names = [ ...imports ].map(([ key, value ]) => esm(key, value));
 
         out.push(`import { ${names.join(", ")} } from "${slash(source)}";`);
@@ -305,7 +313,7 @@ exports.transform = (file, processor, opts = {}) => {
 
     if(namedExports.length) {
         out.push("");
-        
+
         out.push(dedent(`
         export {
             ${namedExports.join(",\n")}
diff --git a/packages/css-to-js/package.json b/packages/css-to-js/package.json
index c8e2d42a..5e81b9a5 100644
--- a/packages/css-to-js/package.json
+++ b/packages/css-to-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@modular-css/css-to-js",
-  "version": "29.0.3",
+  "version": "29.0.4",
   "description": "modular-css powered conversion of CSS to JavaScript",
   "main": "./css-to-js.js",
   "repository": {


Test patch:
diff --git a/packages/css-to-js/test/__snapshots__/api.test.js.snap b/packages/css-to-js/test/__snapshots__/api.test.js.snap
index 4a94f6b2..a99a1891 100644
--- a/packages/css-to-js/test/__snapshots__/api.test.js.snap
+++ b/packages/css-to-js/test/__snapshots__/api.test.js.snap
@@ -288,6 +288,17 @@ export {
 };
 `;
 
+exports[`@modular-css/css-to-js API should represent global composition from external resources: code 1`] = `
+const a = "foo" + " " + "mc74f3fa7b_a";
+export default {
+    a
+};
+
+export {
+    a
+};
+`;
+
 exports[`@modular-css/css-to-js API should represent local @values: code 1`] = `
 const v = "#00F";
 const $values = {
diff --git a/packages/css-to-js/test/api.test.js b/packages/css-to-js/test/api.test.js
index 5efb5c48..50e9a92b 100644
--- a/packages/css-to-js/test/api.test.js
+++ b/packages/css-to-js/test/api.test.js
@@ -85,6 +85,17 @@ describe("@modular-css/css-to-js API", () => {
         expect(namedExports).toEqual([ "b" ]);
     });
 
+    it("should represent global composition from external resources", async () => {
+        const processor = new Processor({ resolvers });
+
+        await processor.string("./a.css", `.a { composes: global(foo); color: red }`);
+
+        const { code, namedExports } = transform("./a.css", processor, { namedExports : false });
+
+        expect(code).toMatchSnapshot("code");
+        expect(namedExports).toEqual([ "a" ]);
+    });
+
     it("should use relative imports when requested", async () => {
         const processor = new Processor({ resolvers });
 
@@ -175,7 +186,7 @@ describe("@modular-css/css-to-js API", () => {
         expect(code).toMatchSnapshot("code");
         expect(namedExports).toEqual([ "$values", "b" ]);
     });
-    
+
     it("should represent external @values aliased to local @values", async () => {
         const processor = new Processor({ resolvers });
 
diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap
index 7d9301d1..fb3b5d00 100644
--- a/packages/rollup/test/__snapshots__/rollup.test.js.snap
+++ b/packages/rollup/test/__snapshots__/rollup.test.js.snap
@@ -1,5 +1,18 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`/rollup.js Exports option should generate valid composition output when global() is used 1`] = `
+{
+  "entry": "
+const out = "foo" + " " + "mc_out";
+var style = {
+    out
+};
+
+console.log(style);
+",
+}
+`;
+
 exports[`/rollup.js Exports option should generated valid JS even when identifiers aren't 1`] = `
 {
   "entry": "
diff --git a/packages/rollup/test/rollup.test.js b/packages/rollup/test/rollup.test.js
index 3c1938f1..06c682f1 100644
--- a/packages/rollup/test/rollup.test.js
+++ b/packages/rollup/test/rollup.test.js
@@ -559,6 +559,21 @@ describe("/rollup.js", () => {
                 })
             ).toMatchRollupCodeSnapshot();
         });
+        
+        it("should generate valid composition output when global() is used", async () => {
+            const bundle = await rollup({
+                input   : require.resolve("./specimens/composes-from-global/entry.js"),
+                plugins : [
+                    createPlugin(),
+                ],
+            });
+
+            expect(
+                await bundle.generate({
+                    format,
+                })
+            ).toMatchRollupCodeSnapshot();
+        });
     });
 
     describe("styleExport option", () => {
diff --git a/packages/rollup/test/specimens/composes-from-global/entry.js b/packages/rollup/test/specimens/composes-from-global/entry.js
new file mode 100644
index 00000000..e989a1fb
--- /dev/null
+++ b/packages/rollup/test/specimens/composes-from-global/entry.js
@@ -0,0 +1,3 @@
+import style from "./one.css";
+
+console.log(style);
diff --git a/packages/rollup/test/specimens/composes-from-global/one.css b/packages/rollup/test/specimens/composes-from-global/one.css
new file mode 100644
index 00000000..3c8067a7
--- /dev/null
+++ b/packages/rollup/test/specimens/composes-from-global/one.css
@@ -0,0 +1,3 @@
+.out {
+    composes: global(foo);
+}