Jelajahi Sumber

[web] Add custom stylelint rule for catching themed variables in :root selector (#34571)

* [web] Add custom stylelint rule for catching themed variables in root selector

* [web] Add lint rule for themed custom property naming

GitOrigin-RevId: 989452e26e427a93f9276ecf011ab7434f96efb1
Mathias Jakobsen 2 bulan lalu
induk
melakukan
e2e01090a6

+ 15 - 2
services/web/.stylelintrc.json

@@ -1,10 +1,23 @@
 {
   "extends": ["stylelint-config-standard-scss"],
+  "plugins": [
+    "./stylelint-rules/no-themed-vars-in-root.mjs",
+    "./stylelint-rules/themed-custom-property-suffix.mjs"
+  ],
   "rules": {
     "function-url-quotes": null,
     "no-descending-specificity": null,
     "scss/at-extend-no-missing-placeholder": null,
     "scss/operator-no-newline-after": null,
-    "property-no-vendor-prefix": [true, { "ignoreProperties": ["mask-image"] }]
-  }
+    "property-no-vendor-prefix": [true, { "ignoreProperties": ["mask-image"] }],
+    "overleaf/no-themed-vars-in-root": true
+  },
+  "overrides": [
+    {
+      "files": ["frontend/stylesheets/abstracts/themes-common-variables.scss"],
+      "rules": {
+        "overleaf/themed-custom-property-suffix": true
+      }
+    }
+  ]
 }

+ 0 - 2
services/web/frontend/stylesheets/abstracts/themes-common-variables.scss

@@ -1,7 +1,6 @@
 /* ====== Semantic CSS color variables that adjust depending on the current theme ====== */
 
 :root {
-  --editor-border-color: var(--neutral-80);
   --bg-primary-themed: var(--bg-dark-primary);
   --bg-secondary-themed: var(--bg-dark-secondary);
   --bg-tertiary-themed: var(--bg-dark-tertiary);
@@ -30,7 +29,6 @@
 }
 
 @include theme('light') {
-  --editor-border-color: var(--neutral-20);
   --bg-primary-themed: var(--bg-light-primary);
   --bg-secondary-themed: var(--bg-light-secondary);
   --bg-tertiary-themed: var(--bg-light-tertiary);

+ 55 - 0
services/web/stylelint-rules/no-themed-vars-in-root.mjs

@@ -0,0 +1,55 @@
+import stylelint from 'stylelint'
+
+const ruleName = 'overleaf/no-themed-vars-in-root'
+
+const messages = stylelint.utils.ruleMessages(ruleName, {
+  rejected: (variable, prop) =>
+    `Unexpected themed variable "${variable}" in the value of "${prop}" inside a ":root" block. ` +
+    'The "data-theme" attribute is set on <body>, not <html>, so a themed variable referenced at ' +
+    '":root" resolves against <html> (always the dark default) and never follows light mode. Declare ' +
+    'this custom property on a descendant selector (e.g. the component container) so the themed ' +
+    'variable resolves within the themed subtree.',
+})
+
+// Matches var(--foo-themed), capturing the variable name.
+const THEMED_VAR_RE = /var\(\s*(--[\w-]*-themed)\s*[,)]/g
+
+/** @type {import('stylelint').Rule} */
+const rule = primary => {
+  return (root, result) => {
+    const validOptions = stylelint.utils.validateOptions(result, ruleName, {
+      actual: primary,
+      possible: [true, false],
+    })
+    if (!validOptions || !primary) {
+      return
+    }
+
+    root.walkRules(node => {
+      const targetsRoot = node.selector
+        .split(',')
+        .map(selector => selector.trim())
+        .some(selector => selector === ':root')
+      if (!targetsRoot) {
+        return
+      }
+
+      node.walkDecls(decl => {
+        for (const [, variable] of decl.value.matchAll(THEMED_VAR_RE)) {
+          stylelint.utils.report({
+            result,
+            ruleName,
+            node: decl,
+            word: variable,
+            message: messages.rejected(variable, decl.prop),
+          })
+        }
+      })
+    })
+  }
+}
+
+rule.ruleName = ruleName
+rule.messages = messages
+
+export default stylelint.createPlugin(ruleName, rule)

+ 42 - 0
services/web/stylelint-rules/themed-custom-property-suffix.mjs

@@ -0,0 +1,42 @@
+import stylelint from 'stylelint'
+
+const ruleName = 'overleaf/themed-custom-property-suffix'
+
+const messages = stylelint.utils.ruleMessages(ruleName, {
+  rejected: prop =>
+    `Custom property "${prop}" must be suffixed with "-themed" (e.g. --bg-secondary-themed), ` +
+    'since this file defines theme-dependent variables.',
+})
+
+/** @type {import('stylelint').Rule} */
+const rule = primary => {
+  return (root, result) => {
+    const validOptions = stylelint.utils.validateOptions(result, ruleName, {
+      actual: primary,
+      possible: [true, false],
+    })
+    if (!validOptions || !primary) {
+      return
+    }
+
+    // Only the declared property name is checked, never var() usages in the
+    // value, so referencing non-themed tokens (e.g. var(--bg-dark-primary)) is fine.
+    root.walkDecls(decl => {
+      if (!decl.prop.startsWith('--') || decl.prop.endsWith('-themed')) {
+        return
+      }
+      stylelint.utils.report({
+        result,
+        ruleName,
+        node: decl,
+        word: decl.prop,
+        message: messages.rejected(decl.prop),
+      })
+    })
+  }
+}
+
+rule.ruleName = ruleName
+rule.messages = messages
+
+export default stylelint.createPlugin(ruleName, rule)