Bug 2067861 - Reject unsupported heading level usage on setting configs r=mstriemer,Standard8
The supported way to set a heading level on a setting is the top-level `headingLevel` field on the config. Setting it another way sort of works today but bypasses the canonical path and makes it harder to build features on top of the field (like the ones in Bug 2044348). This adds an ESLint rule that flags any setting config using `controlAttrs` to set a heading level. When it fails, it shows build as red on a patch and gets blocked from landing. The setting config type is also narrowed so `controlAttrs` can't carry a `headingLevel` key. Once typechecking is enforced on these paths, the eslint rule can be removed and the type-level check alone will catch this. Differential Revision: https://phabricator.services.mozilla.com/D322698
This commit is contained in:
committed by
mkennedy@mozilla.com
parent
23d77249c2
commit
ca24a61ce6
@@ -32,12 +32,24 @@ function expandPaneName(category) {
|
||||
|
||||
/** @import { AttributePart } from "chrome://global/content/vendor/lit.all.mjs" */
|
||||
|
||||
/**
|
||||
* Attributes applied to a setting's control. `headingLevel` variants
|
||||
* are forbidden here — set `headingLevel` at the top level of the
|
||||
* setting config instead.
|
||||
*
|
||||
* @typedef {Record<string, any> & {
|
||||
* headingLevel?: never,
|
||||
* headinglevel?: never,
|
||||
* ".headingLevel"?: never,
|
||||
* }} SettingElementConfigControlAttrs
|
||||
*/
|
||||
|
||||
/**
|
||||
* @typedef {object} SettingElementConfig
|
||||
* @property {string} [id] - The ID for the Setting, this should match the layout id
|
||||
* @property {string} [l10nId] - The Fluent l10n ID for the setting
|
||||
* @property {Record<string, string>} [l10nArgs] - An object containing l10n IDs and their values that will be translated with Fluent
|
||||
* @property {Record<string, any>} [controlAttrs] - An object of additional attributes to be set on the control. These can be used to further customize the control for example a message bar of the warning type, or what dialog a button should open
|
||||
* @property {SettingElementConfigControlAttrs} [controlAttrs] - An object of additional attributes to be set on the control. These can be used to further customize the control for example a message bar of the warning type, or what dialog a button should open
|
||||
* @property {string} [iconSrc] - A path to the icon for the control (if the control supports one)
|
||||
* @property {string} [slot] - The named slot for the control
|
||||
* @property {string} [supportPage] - The SUMO support page slug for the setting
|
||||
|
||||
@@ -496,6 +496,25 @@ let config = [
|
||||
"mozilla/no-newtab-refs-outside-newtab": "error",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "preferences-heading-level-via-config",
|
||||
// Remove once typechecking is enabled and violations fail patches
|
||||
files: [
|
||||
"browser/components/preferences/config/**/*.mjs",
|
||||
"browser/extensions/newtab/lib/AboutPreferences.sys.mjs",
|
||||
],
|
||||
rules: {
|
||||
"no-restricted-syntax": [
|
||||
"error",
|
||||
{
|
||||
selector:
|
||||
":matches(Property[key.name='controlAttrs'], Property[key.value='controlAttrs']) > ObjectExpression > :matches(Property[key.name=/^(?:headinglevel|headingLevel)$/], Property[key.value=/^\\.?(?:headinglevel|headingLevel)$/])",
|
||||
message:
|
||||
"Set heading levels via the top-level `headingLevel` property on the setting config, not through `controlAttrs`.",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
|
||||
...wrapPathsInConfig(subdirConfigs),
|
||||
...wrapPathsInConfig(repositoryGlobals),
|
||||
|
||||
Reference in New Issue
Block a user