Merge pull request #102 from soxhub/fix-sub-expression-strings

Fix issue with sub-expressions in class strings

.changeset/flat-ants-walk.md

@@ -0,0 +1,22 @@
+---
+'ember-scoped-css': patch
+---
+
+Fix an issue with strings in `class` attributes declared via sub-expressions (such as within if statements), were not properly getting scoped.
+
+For example:
+
+```js
+<template>
+  <div class="global-probably {{if @condition "a-local-class"}}">
+    Hello there!
+  </div>
+</template>
+```
+
+When the sibling CSS file only declares `a-local-class`, we would expect that
+
+- `global-probably` remains unchanged
+- `a-local-class` gets hashed
+
+Note that this bug is not fixed for embroider consumers.

ember-scoped-css/build.sh

@@ -9,5 +9,5 @@ pnpm esbuild \
   src/build/app-dependency-loader.js \
   src/lib/scoped-css-preprocessor.js \
   src/runtime/test-support.js \
-  --bundle --outdir=dist --platform=node \
+  --bundle --outdir=dist --platform=node --sourcemap \
   --out-extension:.js=.cjs

ember-scoped-css/src/build/app-js-unplugin.js

@@ -48,6 +48,19 @@ export default createUnplugin(({ appDir }) => {
       );
     },
 
+    /**
+     * This whole thing is error prone, and we should switch the embroider
+     * technique to using a babel plugin.
+     *
+     * Using a webpack plugin is too late in the process.
+     * We don't want to be dealing with wire-format, because
+     * - it's not public API
+     * - it can change as ember-source is upgraded
+     * - the numbers are not "stable", in that they are bitwise anded and ored
+     *   together so that storage is efficient -- which means we'd need to know
+     *   all the opcodes and appropriately | / & to deconstruct appropriately.
+     * @returns
+     */
     async transform(code, id) {
       const cssPath = id.replace(/(\.js)|(\.hbs)/, '.css');
       const postfix = generateHash(cssPath);
@@ -78,6 +91,14 @@ export default createUnplugin(({ appDir }) => {
               .join(' ');
           }
 
+          // replace strings in if conditions
+          // this is brittle, because subexpressions can be deeply nested
+          //
+          // this particular one is <div class="global-probably {{if @condition "a-local-class"}}">"
+          // if (instruction[0] === 15 && Array.isArray(instruction[1]) && instruction[1][0] === 29) {
+
+          // }
+
           // add postfix to tags
           if (
             instruction[0] === 10 &&

ember-scoped-css/src/lib/renameClass.js

@@ -3,7 +3,15 @@ export default function renameClass(className, postfix, classesInCss) {
   const renamedClasses = classes
     .filter((c) => c)
     .map((c) => c.trim())
-    .map((c) => (!classesInCss || classesInCss.has(c) ? c + '_' + postfix : c))
+    .map((c) => {
+      if (!classesInCss || classesInCss.has(c)) {
+        if (c.endsWith(postfix)) return c;
+
+        return c + '_' + postfix;
+      }
+
+      return c;
+    })
     .join(' ');
 
   const renamedWithPreservedSpaces = className.replace(

ember-scoped-css/src/lib/rewriteHbs.js

@@ -19,6 +19,18 @@ export default function rewriteHbs(hbs, classes, tags, postfix) {
               const renamedClass = renameClass(part.chars, postfix, classes);
 
               part.chars = renamedClass;
+            } else if (part.type === 'MustacheStatement') {
+              recast.traverse(part, {
+                StringLiteral(node) {
+                  const renamedClass = renameClass(
+                    node.value,
+                    postfix,
+                    classes,
+                  );
+
+                  node.value = renamedClass;
+                },
+              });
             }
           }
         }