Fix self references in CSS module JS assets (#9080)

packages/core/core/src/AssetGraph.js

@@ -455,6 +455,10 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
         let dependentAsset = assetsByKey.get(dep.specifier);
         if (dependentAsset) {
           dependentAssets.push(dependentAsset);
+          if (dependentAsset.id === asset.id) {
+            // Don't orphan circular dependencies.
+            isDirect = true;
+          }
         }
       }
       let id = this.addNode(nodeFromAsset(asset));

packages/core/core/src/public/Asset.js

@@ -271,6 +271,19 @@ export class MutableAsset extends BaseAsset implements IMutableAsset {
     this.#asset.value.sideEffects = sideEffects;
   }
 
+  get uniqueKey(): ?string {
+    return this.#asset.value.uniqueKey;
+  }
+
+  set uniqueKey(uniqueKey: ?string): void {
+    if (this.#asset.value.uniqueKey != null) {
+      throw new Error(
+        "Cannot change an asset's uniqueKey after it has been set.",
+      );
+    }
+    this.#asset.value.uniqueKey = uniqueKey;
+  }
+
   get symbols(): IMutableAssetSymbols {
     return new MutableAssetSymbols(this.#asset.options, this.#asset.value);
   }

packages/core/integration-tests/test/css-modules.js

@@ -715,4 +715,38 @@ describe('css modules', () => {
     assert.deepEqual(res[0][0], 'mainJs');
     assert(res[0][1].includes('container') && res[0][1].includes('expand'));
   });
+
+  it('should allow css modules to be shared between targets', async function () {
+    let b = await bundle([
+      path.join(__dirname, '/integration/css-module-self-references/a'),
+      path.join(__dirname, '/integration/css-module-self-references/b'),
+    ]);
+
+    assertBundles(b, [
+      {
+        name: 'main.css',
+        assets: ['bar.module.css'],
+      },
+      {
+        name: 'main.css',
+        assets: ['bar.module.css'],
+      },
+      {
+        name: 'main.js',
+        assets: ['index.js', 'bar.module.css'],
+      },
+      {
+        name: 'main.js',
+        assets: ['index.js', 'bar.module.css'],
+      },
+      {
+        name: 'module.js',
+        assets: ['index.js', 'bar.module.css'],
+      },
+      {
+        name: 'module.js',
+        assets: ['index.js', 'bar.module.css'],
+      },
+    ]);
+  });
 });

packages/core/integration-tests/test/integration/css-module-self-references/a/index.js

@@ -0,0 +1,3 @@
+import foo from '../bar.module.css';
+
+console.log('a', foo);

packages/core/integration-tests/test/integration/css-module-self-references/a/package.json

@@ -0,0 +1,5 @@
+{
+  "main": "dist/main.js",
+  "module": "dist/module.js",
+  "source": "index.js"
+}

packages/core/integration-tests/test/integration/css-module-self-references/b/index.js

@@ -0,0 +1,3 @@
+import foo from '../bar.module.css';
+
+console.log('b', foo);

packages/core/integration-tests/test/integration/css-module-self-references/b/package.json

@@ -0,0 +1,5 @@
+{
+  "main": "dist/main.js",
+  "module": "dist/module.js",
+  "source": "index.js"
+}

packages/core/integration-tests/test/integration/css-module-self-references/bar.module.css

@@ -0,0 +1,8 @@
+.foo {
+  composes: composed;
+  color: white;
+}
+
+.composed {
+  background: pink;
+}

packages/core/types/index.js

@@ -749,6 +749,12 @@ export interface MutableAsset extends BaseAsset {
    * This is initially set by the resolver, but can be overridden by transformers.
    */
   sideEffects: boolean;
+  /**
+   * When a transformer returns multiple assets, it can give them unique keys to identify them.
+   * This can be used to find assets during packaging, or to create dependencies between multiple
+   * assets returned by a transformer by using the unique key as the dependency specifier.
+   */
+  uniqueKey: ?string;
   /** The symbols that the asset exports. */
   +symbols: MutableAssetSymbols;
 

packages/packagers/css/src/CSSPackager.js

@@ -36,6 +36,7 @@ export default (new Packager({
           // Hoist unresolved external dependencies (i.e. http: imports)
           if (
             node.value.priority === 'sync' &&
+            !bundleGraph.isDependencySkipped(node.value) &&
             !bundleGraph.getResolvedAsset(node.value, bundle)
           ) {
             hoistedImports.push(node.value.specifier);

packages/transformers/css/src/CSSTransformer.js

@@ -196,6 +196,8 @@ export default (new Transformer({
         locals.set(exports[key].name, key);
       }
 
+      asset.uniqueKey ??= asset.id;
+
       let seen = new Set();
       let add = key => {
         if (seen.has(key)) {
@@ -209,13 +211,16 @@ export default (new Transformer({
         for (let ref of e.composes) {
           s += ' ';
           if (ref.type === 'local') {
-            add(nullthrows(locals.get(ref.name)));
-            s +=
-              '${' +
-              `module.exports[${JSON.stringify(
-                nullthrows(locals.get(ref.name)),
-              )}]` +
-              '}';
+            let exported = nullthrows(locals.get(ref.name));
+            add(exported);
+            s += '${' + `module.exports[${JSON.stringify(exported)}]` + '}';
+            asset.addDependency({
+              specifier: nullthrows(asset.uniqueKey),
+              specifierType: 'esm',
+              symbols: new Map([
+                [exported, {local: ref.name, isWeak: false, loc: null}],
+              ]),
+            });
           } else if (ref.type === 'global') {
             s += ref.name;
           } else if (ref.type === 'dependency') {
@@ -242,6 +247,13 @@ export default (new Transformer({
         // to the JS so the symbol is retained during tree-shaking.
         if (e.isReferenced) {
           s += `module.exports[${JSON.stringify(key)}];\n`;
+          asset.addDependency({
+            specifier: nullthrows(asset.uniqueKey),
+            specifierType: 'esm',
+            symbols: new Map([
+              [key, {local: exports[key].name, isWeak: false, loc: null}],
+            ]),
+          });
         }
 
         js += s;

packages/transformers/js/src/JSTransformer.js

@@ -806,8 +806,14 @@ export default (new Transformer({
           });
         }
 
+        // Use the asset id as a unique key if one has not already been set.
+        // This lets us create a dependency on the asset itself by using it as a specifier.
+        // Using the unique key ensures that the dependency always resolves to the correct asset,
+        // even if it came from a transformer that produced multiple assets (e.g. css modules).
+        // Also avoids needing a resolution request.
+        asset.uniqueKey ||= asset.id;
         asset.addDependency({
-          specifier: `./${path.basename(asset.filePath)}`,
+          specifier: asset.uniqueKey,
           specifierType: 'esm',
           symbols,
         });