diff --git a/change/@fluentui-eslint-plugin-9788e3e7-808d-4201-b04e-cead71f27b9e.json b/change/@fluentui-eslint-plugin-9788e3e7-808d-4201-b04e-cead71f27b9e.json new file mode 100644 index 0000000000000..2055254994b8f --- /dev/null +++ b/change/@fluentui-eslint-plugin-9788e3e7-808d-4201-b04e-cead71f27b9e.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: rule to ban usage of instanceof HTMLElement", + "packageName": "@fluentui/eslint-plugin", + "email": "bernardo.sunderhus@gmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-utilities-851262b0-ac71-4e89-ae48-e53733715e8a.json b/change/@fluentui-react-utilities-851262b0-ac71-4e89-ae48-e53733715e8a.json new file mode 100644 index 0000000000000..b189a9ecaa06c --- /dev/null +++ b/change/@fluentui-react-utilities-851262b0-ac71-4e89-ae48-e53733715e8a.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "chore: exports isHTMLElement as public", + "packageName": "@fluentui/react-utilities", + "email": "bernardo.sunderhus@gmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 94eb1a0f9362f..cf2f685e12a42 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -170,3 +170,27 @@ const context = React.createContext({ someValue: undefined }); import * as React from 'react'; const context = React.createContext(undefined); ``` + +### `ban-instanceof-html-element` + +Bans usage of `instanceof HTMLElement` binary expressions as they might cause problems on [multiple realms](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_realms) environments. + +The alternative is to use `isHTMLElement` helper method provided by `@fluentui/react-utilities` packages, since that method does the proper verifications to ensure proper instance comparison. + +**❌ Don't** + +```ts +event.target instanceof HTMLElement; + +event.target instanceof HTMLInputElement; +``` + +**✅ Do** + +```ts +import { isHTMLElement } from '@fluentui/react-components'; + +isHTMLElement(event.target); + +isHTMLElement(event.target, { constructorName: 'HTMLInputElement' }); +``` diff --git a/packages/eslint-plugin/src/configs/react.js b/packages/eslint-plugin/src/configs/react.js index 441a1997ee8d0..3a45f92585c14 100644 --- a/packages/eslint-plugin/src/configs/react.js +++ b/packages/eslint-plugin/src/configs/react.js @@ -18,6 +18,7 @@ const v9PackageDeps = Object.keys( module.exports = { extends: [path.join(__dirname, 'base'), path.join(__dirname, 'react-config')], rules: { + '@fluentui/ban-instanceof-html-element': ['error'], '@fluentui/no-context-default-value': [ 'error', { diff --git a/packages/eslint-plugin/src/index.js b/packages/eslint-plugin/src/index.js index eb1d2dcd3e4a7..eae81f75a8452 100644 --- a/packages/eslint-plugin/src/index.js +++ b/packages/eslint-plugin/src/index.js @@ -11,6 +11,7 @@ module.exports = { rules: { 'ban-imports': require('./rules/ban-imports'), 'ban-context-export': require('./rules/ban-context-export'), + 'ban-instanceof-html-element': require('./rules/ban-instanceof-html-element'), 'deprecated-keyboard-event-props': require('./rules/deprecated-keyboard-event-props'), 'max-len': require('./rules/max-len'), 'no-global-react': require('./rules/no-global-react'), diff --git a/packages/eslint-plugin/src/rules/ban-instanceof-html-element/index.js b/packages/eslint-plugin/src/rules/ban-instanceof-html-element/index.js new file mode 100644 index 0000000000000..d68b821f0fff3 --- /dev/null +++ b/packages/eslint-plugin/src/rules/ban-instanceof-html-element/index.js @@ -0,0 +1,115 @@ +// @ts-check +const { AST_NODE_TYPES } = require('@typescript-eslint/experimental-utils'); +const createRule = require('../../utils/createRule'); + +/** + * @typedef {import("@typescript-eslint/types/dist/ts-estree").BinaryExpression} BinaryExpression + * @typedef {import("@fluentui/react-utilities/src/utils/isHTMLElement").HTMLElementConstructorName} HTMLElementConstructorName + * + */ + +module.exports = createRule({ + name: 'ban-instanceof-html-element', + meta: { + type: 'problem', + docs: { + description: 'Ban usage of instanceof HTMLElement comparison', + category: 'Possible Errors', + recommended: 'error', + }, + messages: { + invalidBinaryExpression: 'instanceof {{right}} should be avoided, use isHTMLElement instead.', + }, + fixable: 'code', + schema: [], + }, + defaultOptions: [], + create: context => ({ + /** + * @param {BinaryExpression} binaryExpression + */ + // eslint-disable-next-line @typescript-eslint/naming-convention + BinaryExpression(binaryExpression) { + if ( + binaryExpression.operator === 'instanceof' && + binaryExpression.right.type === AST_NODE_TYPES.Identifier && + constructorNamesSet.has(binaryExpression.right.name) + ) { + context.report({ + node: binaryExpression, + messageId: 'invalidBinaryExpression', + data: { + right: binaryExpression.right.name, + }, + }); + } + }, + }), +}); + +/** @type {HTMLElementConstructorName[]} */ +const constructorNames = [ + 'HTMLElement', + 'HTMLAnchorElement', + 'HTMLAreaElement', + 'HTMLAudioElement', + 'HTMLBaseElement', + 'HTMLBodyElement', + 'HTMLBRElement', + 'HTMLButtonElement', + 'HTMLCanvasElement', + 'HTMLDataElement', + 'HTMLDataListElement', + 'HTMLDetailsElement', + 'HTMLDialogElement', + 'HTMLDivElement', + 'HTMLDListElement', + 'HTMLEmbedElement', + 'HTMLFieldSetElement', + 'HTMLFormElement', + 'HTMLHeadingElement', + 'HTMLHeadElement', + 'HTMLHRElement', + 'HTMLHtmlElement', + 'HTMLIFrameElement', + 'HTMLImageElement', + 'HTMLInputElement', + 'HTMLModElement', + 'HTMLLabelElement', + 'HTMLLegendElement', + 'HTMLLIElement', + 'HTMLLinkElement', + 'HTMLMapElement', + 'HTMLMetaElement', + 'HTMLMeterElement', + 'HTMLObjectElement', + 'HTMLOListElement', + 'HTMLOptGroupElement', + 'HTMLOptionElement', + 'HTMLOutputElement', + 'HTMLParagraphElement', + 'HTMLParamElement', + 'HTMLPreElement', + 'HTMLProgressElement', + 'HTMLQuoteElement', + 'HTMLSlotElement', + 'HTMLScriptElement', + 'HTMLSelectElement', + 'HTMLSourceElement', + 'HTMLSpanElement', + 'HTMLStyleElement', + 'HTMLTableElement', + 'HTMLTableColElement', + 'HTMLTableRowElement', + 'HTMLTableSectionElement', + 'HTMLTemplateElement', + 'HTMLTextAreaElement', + 'HTMLTimeElement', + 'HTMLTitleElement', + 'HTMLTrackElement', + 'HTMLUListElement', + 'HTMLVideoElement', +]; + +/** @type {Set} */ +const constructorNamesSet = new Set(constructorNames); diff --git a/packages/eslint-plugin/src/rules/ban-instanceof-html-element/index.test.js b/packages/eslint-plugin/src/rules/ban-instanceof-html-element/index.test.js new file mode 100644 index 0000000000000..5d12eca7289d4 --- /dev/null +++ b/packages/eslint-plugin/src/rules/ban-instanceof-html-element/index.test.js @@ -0,0 +1,38 @@ +// @ts-nocheck +const { ESLintUtils } = require('@typescript-eslint/experimental-utils'); +const rule = require('./index'); + +const ruleTester = new ESLintUtils.RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('ban-instanceof-htmlelement', rule, { + valid: [ + { + code: ` + event.currentTarget instanceof Object + `, + }, + { + code: ` + isHTMLElement(event.currentTarget) + `, + }, + ], + invalid: [ + { + code: ` + event.currentTarget instanceof HTMLElement + event.currentTarget instanceof HTMLInputElement + `, + errors: [{ messageId: 'invalidBinaryExpression' }, { messageId: 'invalidBinaryExpression' }], + }, + { + code: ` + if (event.currentTarget instanceof HTMLElement) {} + if (event.currentTarget instanceof HTMLInputElement) {} + `, + errors: [{ messageId: 'invalidBinaryExpression' }, { messageId: 'invalidBinaryExpression' }], + }, + ], +}); diff --git a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningImperativeAnchorTarget.stories.tsx b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningImperativeAnchorTarget.stories.tsx index bcefde074aaaa..2ac7ac873eafd 100644 --- a/packages/react-components/react-components/stories/Concepts/Positioning/PositioningImperativeAnchorTarget.stories.tsx +++ b/packages/react-components/react-components/stories/Concepts/Positioning/PositioningImperativeAnchorTarget.stories.tsx @@ -29,8 +29,9 @@ export const ImperativeAnchorTarget = () => { setOpen(true); }, []); - const onMouseLeave = React.useCallback((e: React.MouseEvent) => { - if (e.relatedTarget instanceof HTMLElement && e.relatedTarget.getAttribute('role') === 'tooltip') { + const onMouseLeave = React.useCallback((event: React.MouseEvent) => { + const target = event.relatedTarget as HTMLElement; + if (target && target.getAttribute('role') === 'tooltip') { return; } setOpen(false); diff --git a/packages/react-components/react-utilities/etc/react-utilities.api.md b/packages/react-components/react-utilities/etc/react-utilities.api.md index 02740e6bf77c1..b60ec22357e97 100644 --- a/packages/react-components/react-utilities/etc/react-utilities.api.md +++ b/packages/react-components/react-utilities/etc/react-utilities.api.md @@ -81,7 +81,7 @@ export const IdPrefixProvider: React_2.Provider; // @internal export function isFluentTrigger(element: React_2.ReactElement): element is React_2.ReactElement; -// @internal +// @public export function isHTMLElement(element?: unknown, options?: { constructorName?: ConstructorName; }): element is InstanceType<(typeof globalThis)[ConstructorName]>; diff --git a/packages/react-components/react-utilities/src/utils/isHTMLElement.test.ts b/packages/react-components/react-utilities/src/utils/isHTMLElement.test.ts index 7b070f8fa906f..b6009fff2f17c 100644 --- a/packages/react-components/react-utilities/src/utils/isHTMLElement.test.ts +++ b/packages/react-components/react-utilities/src/utils/isHTMLElement.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable @fluentui/ban-instanceof-html-element */ import { isHTMLElement } from './isHTMLElement'; class CustomHTMLElement { diff --git a/packages/react-components/react-utilities/src/utils/isHTMLElement.ts b/packages/react-components/react-utilities/src/utils/isHTMLElement.ts index c4c63d6221b4d..060e2c8bf5c26 100644 --- a/packages/react-components/react-utilities/src/utils/isHTMLElement.ts +++ b/packages/react-components/react-utilities/src/utils/isHTMLElement.ts @@ -1,15 +1,26 @@ /** - * @internal * Verifies if a given node is an HTMLElement, * this method works seamlessly with frames and elements from different documents * - * This is required as simply using `instanceof` - * might be problematic while operating with [multiple realms](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_realms) + * This is preferred over simply using `instanceof`. + * Since `instanceof` might be problematic while operating with [multiple realms](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_realms) + * + * @example + * ```ts + * isHTMLElement(event.target) && event.target.focus() + * isHTMLElement(event.target, {constructorName: 'HTMLInputElement'}) && event.target.value // some value + * ``` * */ export function isHTMLElement( element?: unknown, - options?: { constructorName?: ConstructorName }, + options?: { + /** + * Can be used to provide a custom constructor instead of `HTMLElement`, + * Like `HTMLInputElement` for example. + */ + constructorName?: ConstructorName; + }, ): element is InstanceType<(typeof globalThis)[ConstructorName]> { const typedElement = element as Node | null | undefined; return Boolean( @@ -18,7 +29,10 @@ export function isHTMLElement