diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 486c5ae..11a2fe6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,7 +68,7 @@ jobs: - name: Preparation uses: ./.github/actions/setup - name: Build package - run: npm run build + run: npm run build --if-present - name: Publish run: npm publish --access public env: diff --git a/CODEOWNERS b/CODEOWNERS index 1aa93fd..e6bf555 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @github/ui-platform-reviewers +* @github/web-systems-reviewers diff --git a/docs/rules/extends-correct-class.md b/docs/rules/extends-correct-class.md index 744f37c..b9bcf26 100644 --- a/docs/rules/extends-correct-class.md +++ b/docs/rules/extends-correct-class.md @@ -9,7 +9,7 @@ The [specification defines the requirements of the superclass for each of these ## Rule Details -This rule enforces that any call to `customElements.define` must be given the correct superclass. If the `extends` option is passed, then the given classes superclass must match the named element. If the `extends` option is not passed, then the given classes superclass must be `HTMLElement`. +This rule enforces that any call to `customElements.define` must be given the correct superclass. If the `extends` option is passed, then the given classes superclass must match the named element. If the `extends` option is not passed, then the given classes superclass must be `HTMLElement` or specified by the [`allowedSuperNames` ESLint option](#allowedSuperNames). 👎 Examples of **incorrect** code for this rule: @@ -33,6 +33,10 @@ customElements.define('foo-bar', class extends HTMLElement {}) customElements.define('foo-bar', class extends HTMLParagraphElement {}, {extends: 'p'}) ``` +### Options + +- `allowedSuperNames` is an array option (default: []) can specify what classes an Autonomous custom element _may_ extend. It is assumed that by using this option, any allowed super name _must_ extend `HTMLElement` in its prototype chain. + ## When Not To Use It If you are comfortable with silent failures when extending types don't match. diff --git a/docs/rules/tag-name-matches-class.md b/docs/rules/tag-name-matches-class.md index 143420b..b17d4f8 100644 --- a/docs/rules/tag-name-matches-class.md +++ b/docs/rules/tag-name-matches-class.md @@ -33,29 +33,29 @@ customElements.define('foo-bar', FooBar) - `customElements.define('foo-bar', FooBar)` -- `["error": {"suffix": 'Element'}]` +- `["error", {"suffix": 'Element'}]` - `customElements.define('foo-bar', FooBar)` - `customElements.define('foo-bar', FooBarElement)` -- `["error": {"suffix": ['Element', 'Component']}]` +- `["error", {"suffix": ['Element', 'Component']}]` - `customElements.define('foo-bar', FooBar)` - `customElements.define('foo-bar', FooBarElement)` - `customElements.define('foo-bar', FooBarComponent)` -- `["error": {"prefix": 'Iron'}]` +- `["error", {"prefix": 'Iron'}]` - `customElements.define('foo-bar', FooBar)` - `customElements.define('foo-bar', IronFooBar)` -- `["error": {"prefix": ['Iron', 'Copper']}]` +- `["error", {"prefix": ['Iron', 'Copper']}]` - `customElements.define('foo-bar', FooBar)` - `customElements.define('foo-bar', IronFooBar)` - `customElements.define('foo-bar', CopperFooBar)` -- `["error": {"prefix": ['Iron', 'Copper'], suffix: ['Element', 'Component']}]` +- `["error", {"prefix": ['Iron', 'Copper'], suffix: ['Element', 'Component']}]` - `customElements.define('foo-bar', FooBar)` - `customElements.define('foo-bar', IronFooBar)` - `customElements.define('foo-bar', IronFooBarElement)` diff --git a/lib/rules/extends-correct-class.js b/lib/rules/extends-correct-class.js index f98f1a4..c5c0b01 100644 --- a/lib/rules/extends-correct-class.js +++ b/lib/rules/extends-correct-class.js @@ -15,12 +15,33 @@ function getExtendsOption(node) { return null } +/** + * Format the array of allowed super names for a given class + * into a string for reporting. + * + * @param {Array} allowedSuperNames + * @example + * formatNames(['HTMLElement']) // 'HTMLElement' + * formatNames(['HTMLElement', 'CustomElement']) // HTMLElement or CustomElement + */ +const formatter = new Intl.ListFormat('en', {style: 'short', type: 'disjunction'}) +function formatNames(allowedSuperNames) { + return formatter.format(allowedSuperNames) +} + module.exports = { meta: { type: 'problem', docs: {description: '', url: require('../url')(module)} }, - schema: [], + schema: [ + { + type: 'object', + properties: { + allowedSuperNames: {type: 'array', items: {type: 'string'}} + } + } + ], create(context) { const classes = new ClassRefTracker(context) return { @@ -31,29 +52,39 @@ module.exports = { const classRef = classes.get(node.arguments[1]) if (!classRef) return const extendsTag = getExtendsOption(node.arguments[2]) - const desiredSuperName = builtInTagMap[extendsTag] || 'HTMLElement' + + // Allowed super names is an array of allowed names. + let allowedSuperNames + if (builtInTagMap[extendsTag]) { + allowedSuperNames = new Set([builtInTagMap[extendsTag]]) + } else { + allowedSuperNames = new Set((context.options[0] || {}).allowedSuperNames || []) + allowedSuperNames.add('HTMLElement') + } + + const formattedNames = formatNames(allowedSuperNames) if (!classRef.superClass) { - return context.report(node.arguments[1], `Custom Element must extend ${desiredSuperName}`) + return context.report(node.arguments[1], `Custom Element must extend ${formattedNames}`) } const superName = classRef.superClass.name - if (superName !== desiredSuperName) { + if (!allowedSuperNames.has(superName)) { const expectedTag = Object.entries(builtInTagMap).find(e => e[1] === superName)?.[0] || null if (extendsTag && !expectedTag) { - context.report(node, `${superName} !== ${desiredSuperName}`) + context.report(node, `${superName} !== ${formattedNames}`) } else if (!extendsTag && expectedTag) { context.report( node, - `Custom Element must extend ${desiredSuperName}, or pass {extends:'${expectedTag}'} as a third argument to define` + `Custom Element must extend ${formattedNames}, or pass {extends:'${expectedTag}'} as a third argument to define` ) } else if (extendsTag !== expectedTag) { context.report( node, `Custom Element extends ${superName} but the definition includes {extends:'${extendsTag}'}. ` + - `Either the Custom Element must extend from ${desiredSuperName}, ` + + `Either the Custom Element must extend from ${formattedNames}, ` + `or the definition must include {extends:'${expectedTag}'}.` ) } else { - context.report(node, `Custom Element must extend ${desiredSuperName} not ${superName}`) + context.report(node, `Custom Element must extend ${formattedNames} not ${superName}`) } } } diff --git a/lib/rules/no-customized-built-in-elements.js b/lib/rules/no-customized-built-in-elements.js index 566cc26..7fb2c91 100644 --- a/lib/rules/no-customized-built-in-elements.js +++ b/lib/rules/no-customized-built-in-elements.js @@ -1,4 +1,6 @@ const s = require('../custom-selectors') +const {builtInTagMap} = require('../tag-names') + module.exports = { meta: { type: 'problem', @@ -8,7 +10,11 @@ module.exports = { create(context) { return { [s.HTMLElementClass](node) { - if (node.superClass && node.superClass.name !== 'HTMLElement') { + if ( + node.superClass && + node.superClass.name !== 'HTMLElement' && + Object.values(builtInTagMap).includes(node.superClass.name) + ) { context.report(node, 'Avoid extending built-in elements') } } diff --git a/lib/rules/no-unchecked-define.js b/lib/rules/no-unchecked-define.js index 807f189..9e1252f 100644 --- a/lib/rules/no-unchecked-define.js +++ b/lib/rules/no-unchecked-define.js @@ -11,7 +11,9 @@ module.exports = { create(context) { definedCustomElements = new Map() return { - [`IfStatement:matches([test.type=UnaryExpression],[test.type=BinaryExpression]) ${s.customElements.get}`](node) { + [`IfStatement:matches([test.type=UnaryExpression],[test.type=BinaryExpression],[test.type=LogicalExpression]) ${s.customElements.get}`]( + node + ) { if (node.parent.type === 'UnaryExpression') { let unaryCounter = 0 let parent = node.parent @@ -22,6 +24,10 @@ module.exports = { if (unaryCounter % 2 !== 0) { definedCustomElements.set(node.arguments[0].value, node) } + } else if (node.parent.type === 'LogicalExpression') { + // Don't do anything. If the parent is a logical expression, we are + // checking for truthiness and we only care about if the element is + // _not_ defined. } else { definedCustomElements.set(node.arguments[0].value, node) } diff --git a/package-lock.json b/package-lock.json index 21557c2..896e42d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,11 +1,11 @@ { "name": "eslint-plugin-custom-elements", - "version": "0.0.4", + "version": "0.0.5", "lockfileVersion": 2, "requires": true, "packages": { "": { - "version": "0.0.4", + "version": "0.0.5", "license": "MIT", "devDependencies": { "@github/prettier-config": "0.0.4", diff --git a/package.json b/package.json index 484ab0d..cb1d873 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-custom-elements", - "version": "0.0.4", + "version": "0.0.5", "description": "A collection of eslint rules for custom-elements", "homepage": "https://github.com/github/eslint-plugin-custom-elements#readme", "bugs": { diff --git a/test/extends-correct-class.js b/test/extends-correct-class.js index 970f878..4ebe398 100644 --- a/test/extends-correct-class.js +++ b/test/extends-correct-class.js @@ -7,7 +7,15 @@ ruleTester.run('extends-correct-class', rule, { valid: [ {code: 'customElements.define("foo-bar", class extends HTMLElement {})'}, {code: 'customElements.define("foo-bar", class extends HTMLDivElement {}, { extends: "div" })'}, - {code: 'customElements.define("foo-bar", class extends HTMLOListElement {}, { extends: `ol` })'} + {code: 'customElements.define("foo-bar", class extends HTMLOListElement {}, { extends: `ol` })'}, + { + code: 'customElements.define("foo-bar", class extends HTMLGitHubElement {})', + options: [{allowedSuperNames: ['HTMLGitHubElement']}] + }, + { + code: 'customElements.define("foo-bar", class extends HTMLElement {})', + options: [{allowedSuperNames: ['HTMLGitHubElement']}] + } ], invalid: [ { @@ -55,6 +63,27 @@ ruleTester.run('extends-correct-class', rule, { type: 'CallExpression' } ] + }, + { + code: 'customElements.define("foo-bar", class extends HTMLGitHubElement {}, { extends: `ol` })', + options: [{allowedSuperNames: ['HTMLGitHubElement']}], + errors: [ + { + message: 'HTMLGitHubElement !== HTMLOListElement', + type: 'CallExpression' + } + ] + }, + { + code: 'customElements.define("foo-bar", class extends HTMLGitHubThreeElement {})', + options: [{allowedSuperNames: ['HTMLGitHubOneElement', 'HTMLGitHubTwoElement']}], + errors: [ + { + message: + 'Custom Element must extend HTMLGitHubOneElement, HTMLGitHubTwoElement, or HTMLElement not HTMLGitHubThreeElement', + type: 'CallExpression' + } + ] } ] }) diff --git a/test/no-customized-built-in-elements.js b/test/no-customized-built-in-elements.js index 6ba1fab..98d328a 100644 --- a/test/no-customized-built-in-elements.js +++ b/test/no-customized-built-in-elements.js @@ -7,7 +7,8 @@ ruleTester.run('no-customized-built-in-elements', rule, { {code: 'class SomeMap extends Map { }'}, {code: 'class FooBarElement { }'}, {code: 'class FooBarElement extends HTMLElement { }'}, - {code: 'const FooBarElement = class extends HTMLElement { }'} + {code: 'const FooBarElement = class extends HTMLElement { }'}, + {code: 'const FooBarElement = class extends HTMLRandomNotBuiltInElement { }'} ], invalid: [ { diff --git a/test/no-unchecked-define.js b/test/no-unchecked-define.js index 1735001..8df7cf6 100644 --- a/test/no-unchecked-define.js +++ b/test/no-unchecked-define.js @@ -7,6 +7,9 @@ ruleTester.run('no-unchecked-define', rule, { { code: 'if (!window.customElements.get("foo-bar")) { window.customElements.define("foo-bar", class extends HTMLElement {}) } ' }, + { + code: 'if (window.customElements && !window.customElements.get("foo-bar")) { window.customElements.define("foo-bar", class extends HTMLElement {}) } ' + }, { code: 'if (!customElements.get("foo-bar")) { window.customElements.define("foo-bar", class extends HTMLElement {}) } ' }, @@ -41,6 +44,16 @@ ruleTester.run('no-unchecked-define', rule, { } ] }, + { + code: 'if (window.customElements && customElements.get("foo-bar")) { window.customElements.define("foo-bar", class extends HTMLElement {}) } ', + errors: [ + { + message: + 'Make sure to wrap customElements.define calls in checks to see if the element has already been defined', + type: 'CallExpression' + } + ] + }, { code: 'if (!customElements.get("bar-foo")) { window.customElements.define("foo-bar", class extends HTMLElement {}) } ', errors: [