From e2e01090a6d8bc936b3be68ed9320de596a1a9e2 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Fri, 12 Jun 2026 13:16:56 +0100 Subject: [PATCH] [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 --- services/web/.stylelintrc.json | 17 +++++- .../abstracts/themes-common-variables.scss | 2 - .../no-themed-vars-in-root.mjs | 55 +++++++++++++++++++ .../themed-custom-property-suffix.mjs | 42 ++++++++++++++ 4 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 services/web/stylelint-rules/no-themed-vars-in-root.mjs create mode 100644 services/web/stylelint-rules/themed-custom-property-suffix.mjs diff --git a/services/web/.stylelintrc.json b/services/web/.stylelintrc.json index 09ff0b393f..83ca4692de 100644 --- a/services/web/.stylelintrc.json +++ b/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 + } + } + ] } diff --git a/services/web/frontend/stylesheets/abstracts/themes-common-variables.scss b/services/web/frontend/stylesheets/abstracts/themes-common-variables.scss index 4edc448dab..0a3538ff1f 100644 --- a/services/web/frontend/stylesheets/abstracts/themes-common-variables.scss +++ b/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); diff --git a/services/web/stylelint-rules/no-themed-vars-in-root.mjs b/services/web/stylelint-rules/no-themed-vars-in-root.mjs new file mode 100644 index 0000000000..fd5c04e5b0 --- /dev/null +++ b/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 , not , so a themed variable referenced at ' + + '":root" resolves against (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) diff --git a/services/web/stylelint-rules/themed-custom-property-suffix.mjs b/services/web/stylelint-rules/themed-custom-property-suffix.mjs new file mode 100644 index 0000000000..5cfbb993ad --- /dev/null +++ b/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)