From 911d30c2a628a06784781f9e9986f6ffd35a2f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Thu, 21 Oct 2021 13:45:11 +0100 Subject: [PATCH 01/11] Only run build script if it is present --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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: From 40f76c2cf68b2e665184e61d9fd25f350344a190 Mon Sep 17 00:00:00 2001 From: Michael Bullington Date: Tue, 8 Feb 2022 12:11:21 -0500 Subject: [PATCH 02/11] feat: :sparkles: Option allowedSuperNames for extends-correct-class #32 --- docs/rules/extends-correct-class.md | 6 +++- lib/rules/extends-correct-class.js | 53 ++++++++++++++++++++++++----- test/extends-correct-class.js | 31 ++++++++++++++++- 3 files changed, 80 insertions(+), 10 deletions(-) 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/lib/rules/extends-correct-class.js b/lib/rules/extends-correct-class.js index f98f1a4..89ed8b4 100644 --- a/lib/rules/extends-correct-class.js +++ b/lib/rules/extends-correct-class.js @@ -15,12 +15,36 @@ 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, CustomElement}' + */ +function formatNames(allowedSuperNames) { + if (allowedSuperNames.length === 1) { + return allowedSuperNames[0] + } + + return `{${allowedSuperNames.join(', ')}}` +} + 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 +55,42 @@ 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 = [builtInTagMap[extendsTag]] + } else { + allowedSuperNames = [...((context.options[0] || {}).allowedSuperNames || [])] + // Make sure to treat this like a set. + if (!allowedSuperNames.includes('HTMLElement')) { + allowedSuperNames.push('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.includes(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/test/extends-correct-class.js b/test/extends-correct-class.js index 970f878..93f1ec9 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, HTMLElement} not HTMLGitHubThreeElement', + type: 'CallExpression' + } + ] } ] }) From 216df45874dcbd0e4f98969221d156bc4083b2cd Mon Sep 17 00:00:00 2001 From: Michael Bullington Date: Tue, 8 Feb 2022 12:14:35 -0500 Subject: [PATCH 03/11] fix: :bug: Cross reference no-customized-built-in-elements with builtInTagMap This may change the actual (not expected) behavior of no-customized-built-in-elements in slight ways --- lib/rules/no-customized-built-in-elements.js | 8 +++++++- test/no-customized-built-in-elements.js | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) 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/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: [ { From 85b171de8a30697c3ba493f8745fd1bccb7cbbbc Mon Sep 17 00:00:00 2001 From: Michael Bullington Date: Fri, 11 Feb 2022 16:40:41 -0500 Subject: [PATCH 04/11] Update lib/rules/extends-correct-class.js This works great! I'll pull down and update the unit tests. Co-authored-by: Keith Cirkel --- lib/rules/extends-correct-class.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/rules/extends-correct-class.js b/lib/rules/extends-correct-class.js index 89ed8b4..7bff965 100644 --- a/lib/rules/extends-correct-class.js +++ b/lib/rules/extends-correct-class.js @@ -24,12 +24,9 @@ function getExtendsOption(node) { * formatNames(['HTMLElement']) // 'HTMLElement' * formatNames(['HTMLElement', 'CustomElement']) // '{HTMLElement, CustomElement}' */ +const formatter = new Intl.ListFormat('en', { style: 'short', type: 'disjunction' }) function formatNames(allowedSuperNames) { - if (allowedSuperNames.length === 1) { - return allowedSuperNames[0] - } - - return `{${allowedSuperNames.join(', ')}}` + return formatter.format(allowedSuperNames) } module.exports = { From 7964ca390745c1d3c9d6fbc4f9129d81b5500d4b Mon Sep 17 00:00:00 2001 From: Michael Bullington Date: Fri, 11 Feb 2022 16:46:04 -0500 Subject: [PATCH 05/11] Switch to Set, update tests for Intl --- lib/rules/extends-correct-class.js | 13 +++++-------- test/extends-correct-class.js | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/rules/extends-correct-class.js b/lib/rules/extends-correct-class.js index 7bff965..36d0244 100644 --- a/lib/rules/extends-correct-class.js +++ b/lib/rules/extends-correct-class.js @@ -24,7 +24,7 @@ function getExtendsOption(node) { * formatNames(['HTMLElement']) // 'HTMLElement' * formatNames(['HTMLElement', 'CustomElement']) // '{HTMLElement, CustomElement}' */ -const formatter = new Intl.ListFormat('en', { style: 'short', type: 'disjunction' }) +const formatter = new Intl.ListFormat('en', {style: 'short', type: 'disjunction'}) function formatNames(allowedSuperNames) { return formatter.format(allowedSuperNames) } @@ -56,13 +56,10 @@ module.exports = { // Allowed super names is an array of allowed names. let allowedSuperNames if (builtInTagMap[extendsTag]) { - allowedSuperNames = [builtInTagMap[extendsTag]] + allowedSuperNames = new Set([builtInTagMap[extendsTag]]) } else { - allowedSuperNames = [...((context.options[0] || {}).allowedSuperNames || [])] - // Make sure to treat this like a set. - if (!allowedSuperNames.includes('HTMLElement')) { - allowedSuperNames.push('HTMLElement') - } + allowedSuperNames = new Set((context.options[0] || {}).allowedSuperNames || []) + allowedSuperNames.add('HTMLElement') } const formattedNames = formatNames(allowedSuperNames) @@ -70,7 +67,7 @@ module.exports = { return context.report(node.arguments[1], `Custom Element must extend ${formattedNames}`) } const superName = classRef.superClass.name - if (!allowedSuperNames.includes(superName)) { + if (!allowedSuperNames.has(superName)) { const expectedTag = Object.entries(builtInTagMap).find(e => e[1] === superName)?.[0] || null if (extendsTag && !expectedTag) { context.report(node, `${superName} !== ${formattedNames}`) diff --git a/test/extends-correct-class.js b/test/extends-correct-class.js index 93f1ec9..4ebe398 100644 --- a/test/extends-correct-class.js +++ b/test/extends-correct-class.js @@ -80,7 +80,7 @@ ruleTester.run('extends-correct-class', rule, { errors: [ { message: - 'Custom Element must extend {HTMLGitHubOneElement, HTMLGitHubTwoElement, HTMLElement} not HTMLGitHubThreeElement', + 'Custom Element must extend HTMLGitHubOneElement, HTMLGitHubTwoElement, or HTMLElement not HTMLGitHubThreeElement', type: 'CallExpression' } ] From 4cbe40f0c5c735c06ee9d138518efbb4687f4403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Wed, 23 Feb 2022 11:33:39 +0000 Subject: [PATCH 06/11] Update comment. --- lib/rules/extends-correct-class.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/extends-correct-class.js b/lib/rules/extends-correct-class.js index 36d0244..c5c0b01 100644 --- a/lib/rules/extends-correct-class.js +++ b/lib/rules/extends-correct-class.js @@ -22,7 +22,7 @@ function getExtendsOption(node) { * @param {Array} allowedSuperNames * @example * formatNames(['HTMLElement']) // 'HTMLElement' - * formatNames(['HTMLElement', 'CustomElement']) // '{HTMLElement, CustomElement}' + * formatNames(['HTMLElement', 'CustomElement']) // HTMLElement or CustomElement */ const formatter = new Intl.ListFormat('en', {style: 'short', type: 'disjunction'}) function formatNames(allowedSuperNames) { From 660d3cb2c7a56a4037de52dd914eeaee535a1360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Wed, 16 Mar 2022 12:06:43 +0000 Subject: [PATCH 07/11] Allow define checks to have logical expressions in if statements --- lib/rules/no-unchecked-define.js | 4 +++- test/no-unchecked-define.js | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-unchecked-define.js b/lib/rules/no-unchecked-define.js index 807f189..3282a98 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 diff --git a/test/no-unchecked-define.js b/test/no-unchecked-define.js index 1735001..61c132c 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 {}) } ' }, From 5a6d6260b29e25d2138634bc8cdd6d12bf7433ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Wed, 16 Mar 2022 14:13:15 +0000 Subject: [PATCH 08/11] Fix options syntax in `tag-name-matches-class` rule --- docs/rules/tag-name-matches-class.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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)` From aefc045b1c6f91c6383b719d50d2d55fda7ae58d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Mon, 21 Mar 2022 11:44:47 +0000 Subject: [PATCH 09/11] Cover LogicalExpressions in `no-unchecked-define` --- lib/rules/no-unchecked-define.js | 4 ++++ test/no-unchecked-define.js | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/rules/no-unchecked-define.js b/lib/rules/no-unchecked-define.js index 3282a98..9e1252f 100644 --- a/lib/rules/no-unchecked-define.js +++ b/lib/rules/no-unchecked-define.js @@ -24,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/test/no-unchecked-define.js b/test/no-unchecked-define.js index 61c132c..8df7cf6 100644 --- a/test/no-unchecked-define.js +++ b/test/no-unchecked-define.js @@ -44,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: [ From 4f8d378e8ce561cf760a1f5701fe2f615618294b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Mon, 21 Mar 2022 12:01:52 +0000 Subject: [PATCH 10/11] Update CODEOWNERS --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index 1aa93fd..e6bf555 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @github/ui-platform-reviewers +* @github/web-systems-reviewers From c00174343182a48967402aa2ded17c226773715a Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Tue, 22 Mar 2022 08:55:57 +0000 Subject: [PATCH 11/11] 0.0.5 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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": {