diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 555c54148ec1..7a6b09c5440c 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -7735,6 +7735,9 @@ if (__EXPERIMENTAL__) { useEffect(() => { onStuff(); }, []); + React.useEffect(() => { + onStuff(); + }, []); } `, }, @@ -7751,6 +7754,9 @@ if (__EXPERIMENTAL__) { useEffect(() => { onStuff(); }, [onStuff]); + React.useEffect(() => { + onStuff(); + }, [onStuff]); } `, errors: [ @@ -7769,6 +7775,32 @@ if (__EXPERIMENTAL__) { useEffect(() => { onStuff(); }, []); + React.useEffect(() => { + onStuff(); + }, [onStuff]); + } + `, + }, + ], + }, + { + message: + 'Functions returned from `useEffectEvent` must not be included in the dependency array. ' + + 'Remove `onStuff` from the list.', + suggestions: [ + { + desc: 'Remove the dependency `onStuff`', + output: normalizeIndent` + function MyComponent({ theme }) { + const onStuff = useEffectEvent(() => { + showNotification(theme); + }); + useEffect(() => { + onStuff(); + }, [onStuff]); + React.useEffect(() => { + onStuff(); + }, []); } `, }, diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 7cb3ef049534..ac8886c77680 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -1368,6 +1368,9 @@ if (__EXPERIMENTAL__) { useEffect(() => { onClick(); }); + React.useEffect(() => { + onClick(); + }); } `, }, @@ -1389,6 +1392,10 @@ if (__EXPERIMENTAL__) { let id = setInterval(() => onClick(), 100); return () => clearInterval(onClick); }, []); + React.useEffect(() => { + let id = setInterval(() => onClick(), 100); + return () => clearInterval(onClick); + }, []); return null; } `, @@ -1408,6 +1415,7 @@ if (__EXPERIMENTAL__) { { code: normalizeIndent` function MyComponent({ theme }) { + // Can receive arguments const onEvent = useEffectEvent((text) => { console.log(text); }); @@ -1415,6 +1423,9 @@ if (__EXPERIMENTAL__) { useEffect(() => { onEvent('Hello world'); }); + React.useEffect(() => { + onEvent('Hello world'); + }); } `, }, diff --git a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts index f0a2ffbda9e1..0721a75e0064 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts @@ -11,7 +11,10 @@ import type { CallExpression, CatchClause, DoWhileStatement, + Expression, + Identifier, Node, + Super, TryStatement, } from 'estree'; @@ -129,6 +132,24 @@ function isInsideTryCatch( return false; } +function getNodeWithoutReactNamespace( + node: Expression | Super, +): Expression | Identifier | Super { + if ( + node.type === 'MemberExpression' && + node.object.type === 'Identifier' && + node.object.name === 'React' && + node.property.type === 'Identifier' && + !node.computed + ) { + return node.property; + } + return node; +} + +function isUseEffectIdentifier(node: Node): boolean { + return node.type === 'Identifier' && node.name === 'useEffect'; +} function isUseEffectEventIdentifier(node: Node): boolean { if (__EXPERIMENTAL__) { return node.type === 'Identifier' && node.name === 'useEffectEvent'; @@ -702,10 +723,11 @@ const rule = { // useEffectEvent: useEffectEvent functions can be passed by reference within useEffect as well as in // another useEffectEvent + // Check all `useEffect` and `React.useEffect`, `useEffectEvent`, and `React.useEffectEvent` + const nodeWithoutNamespace = getNodeWithoutReactNamespace(node.callee); if ( - node.callee.type === 'Identifier' && - (node.callee.name === 'useEffect' || - isUseEffectEventIdentifier(node.callee)) && + (isUseEffectIdentifier(nodeWithoutNamespace) || + isUseEffectEventIdentifier(nodeWithoutNamespace)) && node.arguments.length > 0 ) { // Denote that we have traversed into a useEffect call, and stash the CallExpr for diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index f4150c75570f..664b65bf7d7a 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -1206,6 +1206,14 @@ export default class Store extends EventEmitter<{ } set.add(id); } + + const suspense = this._idToSuspense.get(id); + if (suspense !== undefined) { + // We're reconnecting a node. + if (suspense.name === null) { + suspense.name = this._guessSuspenseName(element); + } + } } break; } @@ -1432,21 +1440,12 @@ export default class Store extends EventEmitter<{ const element = this._idToElement.get(id); if (element === undefined) { - this._throwAndEmitError( - Error( - `Cannot add suspense node "${id}" because no matching element was found in the Store.`, - ), - ); + // This element isn't connected yet. } else { if (name === null) { // The boundary isn't explicitly named. // Pick a sensible default. - // TODO: Use key - const owner = this._idToElement.get(element.ownerID); - if (owner !== undefined) { - // TODO: This is clowny - name = `${owner.displayName || 'Unknown'}>?`; - } + name = this._guessSuspenseName(element); } } @@ -1936,4 +1935,15 @@ export default class Store extends EventEmitter<{ // and for unit testing the Store itself. throw error; } + + _guessSuspenseName(element: Element): string | null { + // TODO: Use key + const owner = this._idToElement.get(element.ownerID); + if (owner !== undefined) { + // TODO: This is clowny + return `${owner.displayName || 'Unknown'}>?`; + } + + return null; + } } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js index 8f1d1cd7586a..e7b705205725 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -34,6 +34,8 @@ export type Props = {}; // TODO Make edits and deletes also use transition API! +const noSourcePromise = Promise.resolve(null); + export default function InspectedElementWrapper(_: Props): React.Node { const {inspectedElementID} = useContext(TreeStateContext); const bridge = useContext(BridgeContext); @@ -59,11 +61,11 @@ export default function InspectedElementWrapper(_: Props): React.Node { ? inspectedElement.stack[0] : null; - const symbolicatedSourcePromise: null | Promise = + const symbolicatedSourcePromise: Promise = React.useMemo(() => { - if (fetchFileWithCaching == null) return Promise.resolve(null); + if (fetchFileWithCaching == null) return noSourcePromise; - if (source == null) return Promise.resolve(null); + if (source == null) return noSourcePromise; const [, sourceURL, line, column] = source; return symbolicateSourceWithCache( @@ -291,7 +293,7 @@ export default function InspectedElementWrapper(_: Props): React.Node {
Loading...
)} - {inspectedElement !== null && symbolicatedSourcePromise != null && ( + {inspectedElement !== null && (