From 8efb5067eb15094ccc3b0d940a39d03b99c65931 Mon Sep 17 00:00:00 2001 From: VibhorCodecianGupta Date: Tue, 15 Dec 2020 20:41:58 +0530 Subject: [PATCH] Added unit tests for injectHookVariableNames --- .../__tests__/injectHookVariableNames-test.js | 260 ++++++++++++++++-- packages/react-devtools-shared/src/utils.js | 4 +- 2 files changed, 243 insertions(+), 21 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/injectHookVariableNames-test.js b/packages/react-devtools-shared/src/__tests__/injectHookVariableNames-test.js index ba2a0642e2eb3..802121b15357f 100644 --- a/packages/react-devtools-shared/src/__tests__/injectHookVariableNames-test.js +++ b/packages/react-devtools-shared/src/__tests__/injectHookVariableNames-test.js @@ -9,18 +9,20 @@ import { checkNodeLocation, isConfirmedHookDeclaration, getFilteredHookASTNodes, + filterMemberWithHookVariableName, } from 'react-devtools-shared/src/utils'; describe('injectHookVariableNamesFunction', () => { + it('should identify variable names in destructed syntax', async done => { - const jsxCode = ` + const componentSnippet = ` const Example = () => { const [count, setCount] = React.useState(1); return count; }; `; - const ast = parse(jsxCode, { + const ast = parse(componentSnippet, { sourceType: 'unambiguous', plugins: ['jsx', 'typescript'], }); @@ -35,14 +37,14 @@ describe('injectHookVariableNamesFunction', () => { }); it('should identify variable names in direct assignment', async done => { - const jsxCode = ` + const componentSnippet = ` const Example = () => { const count = React.useState(1); return count; }; `; - const ast = parse(jsxCode, { + const ast = parse(componentSnippet, { sourceType: 'unambiguous', plugins: ['jsx', 'typescript'], }); @@ -57,19 +59,21 @@ describe('injectHookVariableNamesFunction', () => { }); it('should identify variable names in case of destructured assignment', async done => { - const jsxCode = ` + const componentSnippet = ` const Example = () => { - const count = React.useState(1); - const [x, setX] = count; - return count; + const countState = React.useState(1); + const [count, setCount] = countState; + return countState; }; `; - const ast = parse(jsxCode, { + const ast = parse(componentSnippet, { sourceType: 'unambiguous', plugins: ['jsx', 'typescript'], }); + // hookAstNodes captures lines of interest: 3 & 4 const hookAstNodes = getPotentialHookDeclarationsFromAST(ast); + expect(hookAstNodes).toHaveLength(2); // This line number corresponds to where the hook is present const lineNumber = 3; @@ -78,7 +82,7 @@ describe('injectHookVariableNamesFunction', () => { node => checkNodeLocation(node, lineNumber) && isConfirmedHookDeclaration(node), ); - // Find the nodes that are associated with the React Hook found - in this case we obtain the [x, setX] line + // Find the nodes that are associated with the React Hook found - in this case we obtain the [count, setCount] line const nodesAssociatedWithReactHookASTNode = getFilteredHookASTNodes( potentialReactHookASTNode, hookAstNodes, @@ -90,24 +94,242 @@ describe('injectHookVariableNamesFunction', () => { expect(nodesAssociatedWithReactHookASTNode).toHaveLength(1); const relatedNode = nodesAssociatedWithReactHookASTNode[0]; - // The [x,setX] destructuring is on line 4 + // The [count, setCount] destructuring is on line 4 expect(relatedNode.node.loc.start.line).toBe(4); const hookName = getHookVariableName(relatedNode); - expect(hookName).toBe('x'); + expect(hookName).toBe('count'); done(); }); - it('should identify variable names for multiple hooks in one app', async done => { - const jsxCode = ` - const Example = () => { - const count = React.useState(1); - const [x, setX] = count; - const [count1, setCount1] = useState(0); - return count; + it('should identify variable names in case of assignment from object members', async done => { + const componentSnippet = ` + const Example = () => { + const countState = useState(1); + const count = countState[0]; + const setCount = countState[1]; + return countState; + }; + `; + + const ast = parse(componentSnippet, { + sourceType: 'unambiguous', + plugins: ['jsx', 'typescript'], + }); + // hookAstNodes captures lines of interest: 3, 4 & 5 + const hookAstNodes = getPotentialHookDeclarationsFromAST(ast); + expect(hookAstNodes).toHaveLength(3); + // This line number corresponds to where the hook is present + const lineNumber = 3; + + // Isolate the Hook AST Node + const potentialReactHookASTNode = hookAstNodes.find( + node => + checkNodeLocation(node, lineNumber) && isConfirmedHookDeclaration(node), + ); + // Find the nodes that are associated with the React Hook found - in this case we obtain the lines + // -> const count = countState[0]; + // -> const setCount = countState[1]; + const nodesAssociatedWithReactHookASTNode = getFilteredHookASTNodes( + potentialReactHookASTNode, + hookAstNodes, + 'example-app', + new Map(), + ); + + // Two nodes should be found here + expect(nodesAssociatedWithReactHookASTNode).toHaveLength(2); + const nodeAssociatedWithReactHookASTNode = nodesAssociatedWithReactHookASTNode.filter( + hookPath => filterMemberWithHookVariableName(hookPath) + ); + + // Node containing the variable name should be isolated here + expect(nodeAssociatedWithReactHookASTNode).toHaveLength(1); + const relatedNode = nodeAssociatedWithReactHookASTNode[0]; + + // The const count = countState[0] assignment is on line 4 + expect(relatedNode.node.loc.start.line).toBe(4); + + const hookName = getHookVariableName(relatedNode); + expect(hookName).toBe('count'); + done(); + }) + + it('should identify variable names in case of inline assignment from object members', async done => { + const componentSnippet = ` + const Example = () => { + const countState = useState(1); + const count = countState[0], setCount = countState[1]; + return countState; + }; + `; + + const ast = parse(componentSnippet, { + sourceType: 'unambiguous', + plugins: ['jsx', 'typescript'], + }); + // hookAstNodes captures lines of interest: 3 & 4 + const hookAstNodes = getPotentialHookDeclarationsFromAST(ast); + expect(hookAstNodes).toHaveLength(3); + // This line number corresponds to where the hook is present + const lineNumber = 3; + + // Isolate the Hook AST Node + const potentialReactHookASTNode = hookAstNodes.find( + node => + checkNodeLocation(node, lineNumber) && isConfirmedHookDeclaration(node), + ); + // Find the nodes that are associated with the React Hook found - in this case we obtain the line + // -> const count = countState[0], setCount = countState[1]; + const nodesAssociatedWithReactHookASTNode = getFilteredHookASTNodes( + potentialReactHookASTNode, + hookAstNodes, + 'example-app', + new Map(), + ); + + // Two nodes should be found here + expect(nodesAssociatedWithReactHookASTNode).toHaveLength(2); + const nodeAssociatedWithReactHookASTNode = nodesAssociatedWithReactHookASTNode.filter( + hookPath => filterMemberWithHookVariableName(hookPath) + ); + + // Node containing the variable name should be isolated here + expect(nodeAssociatedWithReactHookASTNode).toHaveLength(1); + const relatedNode = nodeAssociatedWithReactHookASTNode[0]; + + // The const count = countState[0] assignment is on line 4 + expect(relatedNode.node.loc.start.line).toBe(4); + + const hookName = getHookVariableName(relatedNode); + expect(hookName).toBe('count'); + done(); + }) + + it('should default to original variable name in case of repeated references', async done => { + const componentSnippet = ` + const Example = () => { + const countState = useState(1); + const count = countState[0]; + const setCount = countState[1]; + const [anotherCount, setAnotherCount] = countState; + return countState; }; `; + const ast = parse(componentSnippet, { + sourceType: 'unambiguous', + plugins: ['jsx', 'typescript'], + }); + // hookAstNodes captures lines of interest: 3, 4, 5 & 6 + const hookAstNodes = getPotentialHookDeclarationsFromAST(ast); + expect(hookAstNodes).toHaveLength(4); + // This line number corresponds to where the hook is present + const lineNumber = 3; + + // Isolate the Hook AST Node + const potentialReactHookASTNode = hookAstNodes.find( + node => + checkNodeLocation(node, lineNumber) && isConfirmedHookDeclaration(node), + ); + // Find the nodes that are associated with the React Hook found - in this case we obtain the lines + // -> const count = countState[0]; + // -> const setCount = countState[1]; + // -> const [anotherCount, setAnotherCount] = countState; + let nodesAssociatedWithReactHookASTNode = []; + nodesAssociatedWithReactHookASTNode = getFilteredHookASTNodes( + potentialReactHookASTNode, + hookAstNodes, + 'example-app', + new Map(), + ); + + // Three nodes should be found here + expect(nodesAssociatedWithReactHookASTNode).toHaveLength(3); + + // More than 2 nodes indicate there are multiple references of a hook assignment + // In such cases we default to the statement const countState = useState(1); + const hookName = getHookVariableName(potentialReactHookASTNode); + expect(hookName).toBe('countState'); + done(); + }) + + it('should default to original variable name in case of no found references', async done => { + const componentSnippet = ` + const Example = () => { + const countState = useState(1); + return countState; + }; + `; + + const ast = parse(componentSnippet, { + sourceType: 'unambiguous', + plugins: ['jsx', 'typescript'], + }); + // hookAstNodes captures lines of interest: 3 + const hookAstNodes = getPotentialHookDeclarationsFromAST(ast); + expect(hookAstNodes).toHaveLength(1); + + // Only one node of interest found + const hookName = getHookVariableName(hookAstNodes[0]); + expect(hookName).toBe('countState'); + done(); + }) + + it('should ignore non declarative primitive hooks', async done => { + const componentSnippet = ` + const Example = (props, ref) => { + const [flag, toggleFlag] = useState(false); + const inputRef = useRef(); + useDebugValue(flag ? 'Set' : 'Reset'); + useImperativeHandle(ref, () => ({ + focus: () => { + inputRef.current.focus(); + } + })); + useEffect(() => { + toggleFlag(true); + }, []); + useLayoutEffect(() => { + console.log(flag) + }, []); + return ; + }; + `; + + const ast = parse(componentSnippet, { + sourceType: 'unambiguous', + plugins: ['jsx', 'typescript'], + }); + + // hookAstNodes captures lines of interest: 3 & 4 + // Should not capture any of the non declarative primitive hooks + const hookAstNodes = getPotentialHookDeclarationsFromAST(ast); + expect(hookAstNodes).toHaveLength(2); + done(); + }) + + it('should identify variable names for multiple hooks in one app', async done => { + const componentSnippet = ` + const Example = () => { + const countState = React.useState(() => 1); + const [count, setCount] = countState; + const [toggle, setToggle] = useState(false); + return [count, toggle]; + }; + `; done(); }); + + it('should identify variable names for multiple hooks declared inline in one app', async done => { + const componentSnippet = ` + const Example = () => { + const ref = React.useRef(null), [ticker, setTicker] = useState(() => 0); + return [ref, ticker]; + }; + `; + done(); + }); + + // custom hook tests }); diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index bc06d0c90cc6c..cfefdb124802e 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -1308,7 +1308,7 @@ function getHookNodeWithInjectedVariableName(originalHook: HooksNode, nodesAssoc // const stateVariable = someState[0] // const setStateVariable = someState[1] // - // const [number2, setNumber2] = state + // const [number2, setNumber2] = someState // // We assign the state variable for 'someState' to multiple variables, // and hence cannot isolate a unique variable name. In such cases, @@ -1344,7 +1344,7 @@ function injectSubHooksWithVariableNames(hook: HooksTree, sourceMaps: Downloaded * @param {NodePath} hook The AST Node Path for the concerned hook * @return {boolean} */ -function filterMemberWithHookVariableName(hook: NodePath): boolean { +export function filterMemberWithHookVariableName(hook: NodePath): boolean { return hook.node.init.property.type === AST_NODE_TYPES.NUMERIC_LITERAL && hook.node.init.property.value === 0; }