feat(eslint-plugin): adds rule ban-instanceof-html-element (#27178)

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": "[email protected]",
+  "dependentChangeType": "patch"
+}

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": "[email protected]",
+  "dependentChangeType": "patch"
+}

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' });
+```

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',
       {

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'),

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<string>} */
+const constructorNamesSet = new Set(constructorNames);

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' }],
+    },
+  ],
+});

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);

packages/react-components/react-utilities/etc/react-utilities.api.md

@@ -81,7 +81,7 @@ export const IdPrefixProvider: React_2.Provider<string | undefined>;
 // @internal
 export function isFluentTrigger(element: React_2.ReactElement): element is React_2.ReactElement<TriggerProps>;
 
-// @internal
+// @public
 export function isHTMLElement<ConstructorName extends HTMLElementConstructorName = 'HTMLElement'>(element?: unknown, options?: {
     constructorName?: ConstructorName;
 }): element is InstanceType<(typeof globalThis)[ConstructorName]>;

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 {

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<ConstructorName extends HTMLElementConstructorName = 'HTMLElement'>(
   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<ConstructorName extends HTMLElementConstructorName
   );
 }
 
-type HTMLElementConstructorName =
+/**
+ * @internal
+ */
+export type HTMLElementConstructorName =
   | 'HTMLElement'
   | 'HTMLAnchorElement'
   | 'HTMLAreaElement'