DEV: Introduce system for renaming CSS variables (#42733)

From time-to-time, variables may need to be renamed in core. To avoid
breaking existing themes and plugins, this commit introduces a system to
automatically rewrite old variable names to new ones. A list of renames
is maintained in `stylesheets/variable-renames.json`, and is applied
when CSS is compiled. This list also powers a stylelint rule which will
automatically rewrite old names to new names in source code.

Transformations apply to all core/theme/plugin code. For now, the
stylelint rule/autofix applies to core only, but this will be extracted
to `@discourse/lint-configs` in the near future.

When a transformation is applied, it adds a trailing `/* automatically
renamed --old to --new */` comment so that the behavior is
understandable from the browser developer tools.

.skills/discourse-writing-html-css/references/color-and-theming.md

@@ -101,3 +101,10 @@ these to restyle every button of a variant without touching `.btn` selectors.
 `--d-sidebar-animation-time`/`-ease`, etc. The admin layout's sidebar adds `--d-sidebar-admin-*`
 (e.g. `--d-sidebar-admin-background`) in `admin/sidebar.scss`. Override these to retheme the
 sidebar in either context without overriding its internal selectors.
+
+## Renaming variables
+
+From time-to-time, variables may need to be renamed in core. To avoid breaking existing themes and plugins,
+Discourse will automatically rewrite old variable names to new ones. A list of renames is maintained in
+`stylesheets/variable-renames.json`, and is applied when CSS is compiled. This list also powers
+a stylelint rule which will automatically rewrite old names to new names in source code.

app/assets/stylesheets/variable-renames.json

@@ -0,0 +1 @@
+{}

frontend/asset-processor/package.json

@@ -32,6 +32,7 @@
     "polyfill-crypto.getrandomvalues": "^1.0.0",
     "postcss": "^8.5.15",
     "postcss-nesting": "^14.0.0",
+    "postcss-value-parser": "^4.2.0",
     "rolldown": "1.1.2",
     "source-map-js": "^1.2.1",
     "strip-test-selectors": "^0.1.0",

frontend/asset-processor/postcss-variable-renamer.js

@@ -0,0 +1,56 @@
+import valueParser from "postcss-value-parser";
+import renames from "../../app/assets/stylesheets/variable-renames.json";
+
+/**
+ * Rewrites renamed CSS custom properties to their current names, so
+ * stylesheets which set or read an old name keep working. Each rewritten
+ * declaration gets a trailing `automatically renamed` comment as a clue in
+ * dev-tools.
+ */
+
+export default function postcssVariableRenamer(renameMap = renames) {
+  const oldNames = Object.keys(renameMap);
+
+  return {
+    postcssPlugin: "postcss-variable-renamer",
+
+    Declaration(declaration, { Comment }) {
+      const applied = new Map();
+
+      const newProp = renameMap[declaration.prop];
+      if (newProp) {
+        applied.set(declaration.prop, newProp);
+        declaration.prop = newProp;
+      }
+
+      if (oldNames.some((oldName) => declaration.value.includes(oldName))) {
+        let valueChanged = false;
+        const parsed = valueParser(declaration.value);
+
+        parsed.walk((node) => {
+          const newName = node.type === "word" && renameMap[node.value];
+          if (newName) {
+            applied.set(node.value, newName);
+            node.value = newName;
+            valueChanged = true;
+          }
+        });
+
+        if (valueChanged) {
+          declaration.value = parsed.toString();
+        }
+      }
+
+      if (applied.size > 0) {
+        const details = [...applied]
+          .map(([oldName, newName]) => `${oldName} to ${newName}`)
+          .join(", ");
+        declaration.after(
+          new Comment({ text: `automatically renamed ${details}` })
+        );
+      }
+    },
+  };
+}
+
+postcssVariableRenamer.postcss = true;

frontend/asset-processor/postcss-variable-renamer.test.mjs

@@ -0,0 +1,77 @@
+/* eslint-disable qunit/require-expect */
+import postcss from "postcss";
+import { expect, test } from "vitest";
+import renames from "../../app/assets/stylesheets/variable-renames.json";
+import postcssVariableRenamer from "./postcss-variable-renamer.js";
+
+const testMap = { "--old-name": "--new-name" };
+
+function process(css) {
+  return postcss([postcssVariableRenamer(testMap)]).process(css, {
+    from: undefined,
+  }).css;
+}
+
+test("rewrites declarations of a renamed variable", () => {
+  expect(process(":root { --old-name: red; }")).toBe(
+    ":root { --new-name: red; /* automatically renamed --old-name to --new-name */ }"
+  );
+});
+
+test("rewrites var() references", () => {
+  expect(process("a { color: var(--old-name); }")).toBe(
+    "a { color: var(--new-name); /* automatically renamed --old-name to --new-name */ }"
+  );
+});
+
+test("rewrites references inside fallbacks", () => {
+  expect(process("a { color: var(--brand, var(--old-name)); }")).toBe(
+    "a { color: var(--brand, var(--new-name)); /* automatically renamed --old-name to --new-name */ }"
+  );
+});
+
+test("preserves fallback of a renamed reference", () => {
+  expect(process("a { color: var(--old-name, blue); }")).toBe(
+    "a { color: var(--new-name, blue); /* automatically renamed --old-name to --new-name */ }"
+  );
+});
+
+test("leaves longer names alone", () => {
+  const css = "a { color: var(--old-name-hover); }";
+  expect(process(css)).toBe(css);
+});
+
+test("leaves names with a prefix alone", () => {
+  const css = "a { color: var(--theme--old-name); }";
+  expect(process(css)).toBe(css);
+});
+
+test("adds one comment when a declaration sets and reads the same old name", () => {
+  expect(process(":root { --old-name: var(--old-name, red); }")).toBe(
+    ":root { --new-name: var(--new-name, red); /* automatically renamed --old-name to --new-name */ }"
+  );
+});
+
+test("leaves untouched declarations alone", () => {
+  const css = "a { color: var(--primary); }";
+  expect(process(css)).toBe(css);
+});
+
+test("leaves string contents alone", () => {
+  const css = 'a::before { content: "--old-name"; }';
+  expect(process(css)).toBe(css);
+});
+
+test("map has no chains", () => {
+  const oldNames = new Set(Object.keys(renames));
+  for (const newName of Object.values(renames)) {
+    expect(oldNames).not.toContain(newName);
+  }
+});
+
+test("map only contains custom property names", () => {
+  for (const [oldName, newName] of Object.entries(renames)) {
+    expect(oldName).toMatch(/^--[\w-]+$/);
+    expect(newName).toMatch(/^--[\w-]+$/);
+  }
+});

frontend/asset-processor/postcss.js

@@ -5,8 +5,10 @@ import postcss from "postcss";
 import postcssNesting from "postcss-nesting";
 import { browsers } from "../discourse/config/targets";
 import postcssVariablePrefixer from "./postcss-variable-prefixer";
+import postcssVariableRenamer from "./postcss-variable-renamer";
 
 const postCssProcessor = postcss([
+  postcssVariableRenamer(),
   autoprefixer({
     overrideBrowserslist: browsers,
   }),

lib/asset_processor.rb

@@ -10,6 +10,7 @@ class AssetProcessor
       dependency_globs: %w[
         node_modules/.pnpm/lock.yaml
         frontend/asset-processor/**/*.{js,mjs}
+        app/assets/stylesheets/variable-renames.json
         frontend/discourse/lib/babel-transform-module-renames.js
         frontend/discourse/lib/discourse-source-imports.mjs
         frontend/discourse/config/targets.js

lib/stylesheet/manager.rb

@@ -17,6 +17,7 @@ class Stylesheet::Manager
   private_constant :CACHE_PATH
 
   MANIFEST_DIR = "#{Rails.root.join("tmp/cache/assets/#{Rails.env}")}"
+  VARIABLE_RENAMES_PATH = Rails.root.join("app/assets/stylesheets/variable-renames.json")
   THEME_REGEX = /_theme(_rtl)?\z/
   COLOR_SCHEME_STYLESHEET = "color_definitions"
 
@@ -189,6 +190,7 @@ class Stylesheet::Manager
       "#{Rails.root.join("app/assets/stylesheets/**/*.*css")}",
       "#{Rails.root.join("app/assets/images/**/*.*")}",
       "#{Rails.root.join("lib/stylesheet/*.rb")}",
+      VARIABLE_RENAMES_PATH.to_s,
     ]
 
     Discourse.plugins.each do |plugin|

lib/stylesheet/manager/builder.rb

@@ -193,7 +193,7 @@ class Stylesheet::Manager::Builder
   def theme_digest
     Digest::SHA1.hexdigest(
       scss_digest.to_s + color_scheme_digest.to_s + settings_digest + uploads_digest +
-        current_hostname,
+        current_hostname + Stylesheet::Manager.fs_asset_cachebuster,
     )
   end
 

package.json

@@ -21,6 +21,7 @@
     "playwright": "1.59.1",
     "postcss-selector-parser": "^7.1.4",
     "prettier": "3.8.1",
+    "postcss-value-parser": "^4.2.0",
     "stylelint": "17.5.0",
     "typescript": "^5.9.3"
   },

pnpm-lock.yaml

@@ -90,6 +90,9 @@ importers:
       postcss-selector-parser:
         specifier: ^7.1.4
         version: 7.1.4
+      postcss-value-parser:
+        specifier: ^4.2.0
+        version: 4.2.0
       prettier:
         specifier: 3.8.1
         version: 3.8.1
@@ -177,6 +180,9 @@ importers:
       postcss-nesting:
         specifier: ^14.0.0
         version: 14.0.0(postcss@8.5.15)
+      postcss-value-parser:
+        specifier: ^4.2.0
+        version: 4.2.0
       rolldown:
         specifier: 1.1.2
         version: 1.1.2

spec/lib/stylesheet/manager_spec.rb

@@ -317,6 +317,22 @@ RSpec.describe Stylesheet::Manager do
       expect(digest1).not_to eq(digest2)
     end
 
+    it "accounts for the asset cachebuster in theme digests" do
+      theme = Fabricate(:theme)
+
+      builder =
+        Stylesheet::Manager::Builder.new(target: :desktop_theme, theme: theme, manager: manager)
+      digest1 = builder.digest
+
+      Stylesheet::Manager.stubs(:fs_asset_cachebuster).returns("changed")
+
+      builder =
+        Stylesheet::Manager::Builder.new(target: :desktop_theme, theme: theme, manager: manager)
+      digest2 = builder.digest
+
+      expect(digest1).not_to eq(digest2)
+    end
+
     it "can correctly account for settings in theme's components" do
       theme = Fabricate(:theme)
       child = Fabricate(:theme, component: true)
@@ -1130,5 +1146,11 @@ RSpec.describe Stylesheet::Manager do
         expect(new_cachebuster).not_to eq(initial_cachebuster)
       end
     end
+
+    it "includes the variable rename map in its inputs" do
+      expect(Stylesheet::Manager.send(:list_files)).to include(
+        Stylesheet::Manager::VARIABLE_RENAMES_PATH.to_s,
+      )
+    end
   end
 end

stylelint-rules/no-renamed-variables.mjs

@@ -0,0 +1,60 @@
+import { readFileSync } from "node:fs";
+import valueParser from "postcss-value-parser";
+import stylelint from "stylelint";
+
+const renames = JSON.parse(
+  readFileSync(
+    new URL("../app/assets/stylesheets/variable-renames.json", import.meta.url)
+  )
+);
+
+const ruleName = "discourse/no-renamed-variables";
+const messages = stylelint.utils.ruleMessages(ruleName, {
+  renamed: (oldName, newName) => `"${oldName}" was renamed to "${newName}"`,
+});
+
+const ruleFunction = (primaryOption) => {
+  return (root, result) => {
+    if (!primaryOption) {
+      return;
+    }
+
+    root.walkDecls((decl) => {
+      const newProp = renames[decl.prop];
+      if (newProp) {
+        stylelint.utils.report({
+          message: messages.renamed(decl.prop, newProp),
+          node: decl,
+          result,
+          ruleName,
+          word: decl.prop,
+          fix: () => (decl.prop = newProp),
+        });
+      }
+
+      const parsed = valueParser(decl.value);
+      parsed.walk((node) => {
+        const newName = node.type === "word" && renames[node.value];
+        if (newName) {
+          stylelint.utils.report({
+            message: messages.renamed(node.value, newName),
+            node: decl,
+            result,
+            ruleName,
+            word: node.value,
+            fix: () => {
+              node.value = newName;
+              decl.value = parsed.toString();
+            },
+          });
+        }
+      });
+    });
+  };
+};
+
+ruleFunction.ruleName = ruleName;
+ruleFunction.messages = messages;
+ruleFunction.meta = { fixable: true };
+
+export default stylelint.createPlugin(ruleName, ruleFunction);

stylelint.config.mjs

@@ -1,12 +1,19 @@
 import noCoreVariables from "./stylelint-rules/no-core-variables.mjs";
+import noRenamedVariables from "./stylelint-rules/no-renamed-variables.mjs";
 import requireDesignTokens from "./stylelint-rules/require-design-tokens.mjs";
 import ucClassesInWhere from "./stylelint-rules/uc-classes-in-where.mjs";
 
 export default {
   extends: ["@discourse/lint-configs/stylelint"],
-  plugins: [noCoreVariables, requireDesignTokens, ucClassesInWhere],
+  plugins: [
+    noCoreVariables,
+    noRenamedVariables,
+    requireDesignTokens,
+    ucClassesInWhere,
+  ],
   rules: {
     "media-feature-range-notation": "context",
+    "discourse/no-renamed-variables": true,
     "discourse/uc-classes-in-where": true,
   },
   overrides: [