diff --git a/.changeset/quiet-hooks-listen.md b/.changeset/quiet-hooks-listen.md new file mode 100644 index 00000000000..c28e0a744ee --- /dev/null +++ b/.changeset/quiet-hooks-listen.md @@ -0,0 +1,5 @@ +--- +'react-docgen': patch +--- + +Document methods declared with `useCallback` and exposed through `useImperativeHandle` diff --git a/packages/react-docgen/src/__tests__/imperativeHandleCallbacks-test.ts b/packages/react-docgen/src/__tests__/imperativeHandleCallbacks-test.ts new file mode 100644 index 00000000000..7985d096b9b --- /dev/null +++ b/packages/react-docgen/src/__tests__/imperativeHandleCallbacks-test.ts @@ -0,0 +1,97 @@ +import { parse } from '../main.js'; +import { describe, expect, test } from 'vitest'; + +function parseSource(source: string) { + return parse(source, { + filename: 'file.tsx', + babelOptions: { babelrc: false }, + }); +} + +const method = { + name: '_myMethod', + params: [{ name: 'argument', optional: false, type: { name: 'string' } }], + returns: { type: { name: 'number' } }, +}; + +describe('useImperativeHandle methods', () => { + // The component reported in + // https://github.com/reactjs/react-docgen/issues/856, verbatim. + test('documents a useCallback method of a memo(forwardRef) component', () => { + const docs = parseSource(` + import React, { + forwardRef, + memo, + useCallback, + useImperativeHandle, + useMemo, + useRef, + } from 'react'; + + export const MyComponent = + memo(forwardRef((_, ref) => { + + const _myMethod = useCallback((argument:string) : number => {}); + + useImperativeHandle( + ref, + () => ({ + /** myMethod description */ + _myMethod, + }), + [], + ); + + return
; + })); + `); + + expect(docs).toHaveLength(1); + expect(docs[0]).toMatchObject({ + methods: [ + { + ...method, + docblock: 'myMethod description', + description: 'myMethod description', + }, + ], + }); + }); + + test('documents a useCallback method of a forwardRef component', () => { + const docs = parseSource(` + import React, { forwardRef, useCallback, useImperativeHandle } from 'react'; + + export const MyComponent = forwardRef((_, ref) => { + const _myMethod = useCallback((argument: string): number => 1, []); + + useImperativeHandle(ref, () => ({ _myMethod }), []); + + return
; + }); + `); + + expect(docs).toHaveLength(1); + expect(docs[0]).toMatchObject({ methods: [method] }); + }); + + test('documents a plain function method of a memo(forwardRef) component (control)', () => { + // Controls for the wrapper resolution itself: the same component without + // useCallback is documented before this change, so the two tests above + // fail on the unwrapping and not on memo/forwardRef. + const docs = parseSource(` + import React, { forwardRef, memo, useImperativeHandle } from 'react'; + + export const MyComponent = memo(forwardRef((_, ref) => { + const _myMethod = (argument: string): number => 1; + + useImperativeHandle(ref, () => ({ _myMethod }), []); + + return
; + })); + `); + + expect(docs).toHaveLength(1); + expect(docs[0]).toMatchObject({ methods: [method] }); + }); +}); diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts new file mode 100644 index 00000000000..df518c0fce4 --- /dev/null +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -0,0 +1,354 @@ +import { parse } from '../../../tests/utils'; +import componentMethodsHandler from '../componentMethodsHandler.js'; +import DocumentationBuilder from '../../Documentation'; +import type DocumentationMock from '../../__mocks__/Documentation'; +import type { + ClassDeclaration, + FunctionDeclaration, + ObjectExpression, +} from '@babel/types'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; + +vi.mock('../../Documentation.js'); + +describe('componentMethodsHandler useImperativeHandle callbacks', () => { + let documentation: DocumentationBuilder & DocumentationMock; + + beforeEach(() => { + documentation = new DocumentationBuilder() as DocumentationBuilder & + DocumentationMock; + }); + + const wrappedSignature = { + name: 'method', + params: [{ name: 'argument', optional: false, type: { name: 'string' } }], + returns: { type: { name: 'number' } }, + }; + + test.each([ + { + name: 'a directly-declared callback', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: '', + value: 'useCallback((argument: string): number => 1, [])', + imperativeHandle: 'useImperativeHandle', + }, + { + name: 'a callback identifier', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: 'const callback = (argument: string): number => 1;', + value: 'useCallback(callback, [])', + imperativeHandle: 'useImperativeHandle', + }, + { + name: 'a React namespace callback', + imports: "import * as React from 'react';", + setup: '', + value: 'React.useCallback((argument: string): number => 1, [])', + imperativeHandle: 'React.useImperativeHandle', + }, + { + name: 'a renamed useCallback import', + imports: + "import { useCallback as useCb, useImperativeHandle } from 'react';", + setup: '', + value: 'useCb((argument: string): number => 1, [])', + imperativeHandle: 'useImperativeHandle', + }, + ])( + 'extracts a method wrapped with $name', + ({ imports, setup, value, imperativeHandle }) => { + const definition = parse.statementLast(` + ${imports} + function Component() { + ${setup} + const method = ${value}; + ${imperativeHandle}(ref, () => ({ method })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + // The wrapped function's own signature is what gets documented, not + // the useCallback call around it. + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }, + ); + + test('extracts a callback written inline in the handle object', () => { + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + useImperativeHandle(ref, () => ({ + method: useCallback((argument: string): number => 1, []), + })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }); + + test('extracts a callback declared after the imperative handle', () => { + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + useImperativeHandle(ref, () => ({ method })); + const method = useCallback((argument: string): number => 1, []); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }); + + test('carries the docblock of the handle property', () => { + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + const method = useCallback((argument: string): number => 1, []); + useImperativeHandle(ref, () => ({ + /** + * The method + */ + method, + })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject({ + ...wrappedSignature, + docblock: 'The method', + }); + }); + + test.each([ + { + name: 'async', + callback: 'async (argument: string): number => 1', + modifiers: ['async'], + }, + { + name: 'generator', + callback: 'function* (argument: string): number {}', + modifiers: ['generator'], + }, + ])( + 'records the $name modifier of the wrapped function', + ({ callback, modifiers }) => { + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + const method = useCallback(${callback}, []); + useImperativeHandle(ref, () => ({ method })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject({ + name: 'method', + modifiers, + }); + }, + ); + + test('extracts a zero-parameter callback with no return annotation', () => { + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + const method = useCallback(() => {}, []); + useImperativeHandle(ref, () => ({ method })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject({ + name: 'method', + params: [], + returns: null, + }); + }); + + test('documents a plain function exposed through the handle (control)', () => { + // Controls for the imperative handle surface itself: it documented plain + // function values before this change, so the callback cases above fail on + // the unwrapping, not on the handle. + const definition = parse.statementLast(` + import { useImperativeHandle } from 'react'; + function Component() { + const method = (argument: string): number => 1; + useImperativeHandle(ref, () => ({ method })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }); + + test('does not document a lifecycle name exposed as a callback (control)', () => { + // The React lifecycle filter still applies to an unwrapped callback, the + // same way it applies to a plain function property. + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + const render = useCallback((argument: string): number => 1, []); + useImperativeHandle(ref, () => ({ render })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(0); + }); + + // Hooks are only valid inside a function component, so the unwrapping stays + // on the imperative handle path: these surfaces document plain functions and + // keep ignoring useCallback calls. + test('does not document a callback method on an ObjectExpression component (control)', () => { + const definition = parse.expressionLast(` + import { useCallback } from 'react'; + ({ + method: useCallback((argument: string): number => 1, []), + }) + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(0); + }); + + test('does not document a callback method in a statics object (control)', () => { + const definition = parse.expressionLast(` + import { useCallback } from 'react'; + ({ + statics: { + method: useCallback((argument: string): number => 1, []), + }, + }) + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(0); + }); + + test('does not document a callback class property (control)', () => { + const definition = parse.statementLast(` + import React, { useCallback } from 'react'; + class Test extends React.Component { + method = useCallback((argument: string): number => 1, []); + render() { return null; } + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(0); + }); + + test('documents a plain function class property (control)', () => { + // Controls for the class-property surface: it still documents plain + // function values, so the callback class property above is undocumented + // because of the scoping and not because the surface stopped working. + const definition = parse.statementLast(` + import React from 'react'; + class Test extends React.Component { + method = (argument: string): number => 1; + render() { return null; } + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }); + + // Each of these pins one guard the unwrapping must keep: they are already + // undocumented before this change, and must stay undocumented after it. + test.each([ + { + // the call has to resolve to React's useCallback, not any binding of + // that name + name: 'a local function named useCallback (control)', + imports: "import { useImperativeHandle } from 'react';", + setup: + 'function useCallback(fn: unknown, deps: unknown[]) { return fn; }', + value: 'useCallback((argument: string): number => 1, [])', + }, + { + // there has to be a first argument at all + name: 'a useCallback call with no arguments (control)', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: '', + value: 'useCallback()', + }, + { + // the first argument has to resolve to a function + name: 'a useCallback call whose first argument is not a function (control)', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: '', + value: 'useCallback(42, [])', + }, + { + // a spread element is not the callback, even when it spreads one + name: 'a useCallback call whose argument is spread (control)', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: 'const args = [(argument: string): number => 1, []];', + value: 'useCallback(...args)', + }, + { + // the unwrapping is one level deep, not recursive + name: 'a nested useCallback call (control)', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: '', + value: + 'useCallback(useCallback((argument: string): number => 1, []), [])', + }, + { + // only useCallback is unwrapped, not every React builtin that returns + // a function + name: 'a useMemo call returning a function (control)', + imports: "import { useMemo, useImperativeHandle } from 'react';", + setup: '', + value: 'useMemo(() => (argument: string): number => 1, [])', + }, + ])('does not document $name', ({ imports, setup, value }) => { + const definition = parse.statementLast(` + ${imports} + function Component() { + ${setup} + const method = ${value}; + useImperativeHandle(ref, () => ({ method })); + return
; + } + `); + + expect(() => + componentMethodsHandler(documentation, definition), + ).not.toThrow(); + expect(documentation.methods).toHaveLength(0); + }); +}); diff --git a/packages/react-docgen/src/handlers/componentMethodsHandler.ts b/packages/react-docgen/src/handlers/componentMethodsHandler.ts index 2bbcd571473..93b9965002f 100644 --- a/packages/react-docgen/src/handlers/componentMethodsHandler.ts +++ b/packages/react-docgen/src/handlers/componentMethodsHandler.ts @@ -1,6 +1,8 @@ import getMemberValuePath from '../utils/getMemberValuePath.js'; import type { MethodNodePath } from '../utils/getMethodDocumentation.js'; -import getMethodDocumentation from '../utils/getMethodDocumentation.js'; +import getMethodDocumentation, { + resolveToUseCallbackFunction, +} from '../utils/getMethodDocumentation.js'; import isReactComponentClass from '../utils/isReactComponentClass.js'; import isReactComponentMethod from '../utils/isReactComponentMethod.js'; import type Documentation from '../Documentation.js'; @@ -48,6 +50,22 @@ function isMethod(path: NodePath): path is MethodNodePath { return isProbablyMethod && !isReactComponentMethod(path); } +/** + * A method exposed through `useImperativeHandle` is also a method when it is + * wrapped in `useCallback`, which is how such methods are usually memoized. + */ +function isImperativeHandleMethod(path: NodePath): path is MethodNodePath { + if (isMethod(path)) { + return true; + } + + return ( + path.isObjectProperty() && + resolveToUseCallbackFunction(path.get('value') as NodePath) !== null && + !isReactComponentMethod(path) + ); +} + interface TraverseState { readonly scope: Scope | undefined; readonly name: string; @@ -79,6 +97,7 @@ const explodedVisitors = visitors.explode({ interface MethodDefinition { path: MethodNodePath; + isImperativeHandle?: boolean; isStatic?: boolean; } @@ -121,7 +140,7 @@ const explodedImperativeHandleVisitors = // We found the object body, now add all of the properties as methods. definition?.get('properties').forEach((p) => { - if (isMethod(p)) { + if (isImperativeHandleMethod(p)) { state.results.push(p); } }); @@ -166,7 +185,7 @@ function findImperativeHandleMethods( body.traverse(explodedImperativeHandleVisitors, state); - return state.results.map((p) => ({ path: p })); + return state.results.map((p) => ({ path: p, isImperativeHandle: true })); } function findAssignedMethods( @@ -267,7 +286,9 @@ const componentMethodsHandler: Handler = function ( documentation.set( 'methods', methodPaths - .map(({ path: p, isStatic }) => getMethodDocumentation(p, { isStatic })) + .map(({ path: p, isImperativeHandle, isStatic }) => + getMethodDocumentation(p, { isImperativeHandle, isStatic }), + ) .filter(Boolean), ); }; diff --git a/packages/react-docgen/src/utils/getMethodDocumentation.ts b/packages/react-docgen/src/utils/getMethodDocumentation.ts index 3dc0162ea53..119dbc3f8ce 100644 --- a/packages/react-docgen/src/utils/getMethodDocumentation.ts +++ b/packages/react-docgen/src/utils/getMethodDocumentation.ts @@ -14,6 +14,7 @@ import getTSType from './getTSType.js'; import getParameterName from './getParameterName.js'; import getPropertyName from './getPropertyName.js'; import getTypeAnnotation from './getTypeAnnotation.js'; +import isReactBuiltinCall from './isReactBuiltinCall.js'; import resolveToValue from './resolveToValue.js'; import printValue from './printValue.js'; import type { @@ -32,8 +33,42 @@ export type MethodNodePath = | NodePath | NodePath; +export interface MethodOptions { + /** + * Set for methods exposed through `useImperativeHandle`, which are allowed + * to be wrapped in `useCallback`. + */ + isImperativeHandle?: boolean; + isStatic?: boolean; +} + +/** + * Returns the function that a React `useCallback` call wraps, or null when + * the path is not such a call or does not wrap a function. + */ +export function resolveToUseCallbackFunction( + path: NodePath, +): NodePath | null { + const value = resolveToValue(path); + + if (value.isCallExpression() && isReactBuiltinCall(value, 'useCallback')) { + const callback = value.get('arguments')[0]; + + if (callback && !Array.isArray(callback)) { + const wrapped = resolveToValue(callback); + + if (wrapped.isFunction()) { + return wrapped; + } + } + } + + return null; +} + function getMethodFunctionExpression( methodPath: MethodNodePath, + options: MethodOptions, ): NodePath | null { if (methodPath.isClassMethod() || methodPath.isObjectMethod()) { return methodPath; @@ -49,6 +84,12 @@ function getMethodFunctionExpression( return functionExpression; } + // An imperative handle method is documented through the function its + // `useCallback` wraps, because that is where its signature is declared. + if (options.isImperativeHandle) { + return resolveToUseCallbackFunction(potentialFunctionExpression); + } + return null; } @@ -68,9 +109,12 @@ function getMethodParamOptional( return identifier.isIdentifier() ? Boolean(identifier.node.optional) : false; } -function getMethodParamsDoc(methodPath: MethodNodePath): MethodParameter[] { +function getMethodParamsDoc( + methodPath: MethodNodePath, + options: MethodOptions, +): MethodParameter[] { const params: MethodParameter[] = []; - const functionExpression = getMethodFunctionExpression(methodPath); + const functionExpression = getMethodFunctionExpression(methodPath, options); if (functionExpression) { // Extract param types. @@ -106,8 +150,11 @@ function getMethodParamsDoc(methodPath: MethodNodePath): MethodParameter[] { } // Extract flow return type. -function getMethodReturnDoc(methodPath: MethodNodePath): MethodReturn | null { - const functionExpression = getMethodFunctionExpression(methodPath); +function getMethodReturnDoc( + methodPath: MethodNodePath, + options: MethodOptions, +): MethodReturn | null { + const functionExpression = getMethodFunctionExpression(methodPath, options); if (functionExpression && functionExpression.node.returnType) { const returnType = getTypeAnnotation(functionExpression.get('returnType')); @@ -128,7 +175,7 @@ function getMethodReturnDoc(methodPath: MethodNodePath): MethodReturn | null { function getMethodModifiers( methodPath: MethodNodePath, - options: { isStatic?: boolean }, + options: MethodOptions, ): MethodModifier[] { if (methodPath.isAssignmentExpression()) { return ['static']; @@ -146,7 +193,7 @@ function getMethodModifiers( modifiers.push('static'); } - const functionExpression = getMethodFunctionExpression(methodPath); + const functionExpression = getMethodFunctionExpression(methodPath, options); if (functionExpression) { if ( @@ -230,7 +277,7 @@ function getMethodDocblock(methodPath: MethodNodePath): string | null { // or as assignment expression of the form `Component.foo = function() {}` export default function getMethodDocumentation( methodPath: MethodNodePath, - options: { isStatic?: boolean } = {}, + options: MethodOptions = {}, ): MethodDescriptor | null { if ( getMethodAccessibility(methodPath) === 'private' || @@ -247,7 +294,7 @@ export default function getMethodDocumentation( name, docblock: getMethodDocblock(methodPath), modifiers: getMethodModifiers(methodPath, options), - params: getMethodParamsDoc(methodPath), - returns: getMethodReturnDoc(methodPath), + params: getMethodParamsDoc(methodPath, options), + returns: getMethodReturnDoc(methodPath, options), }; }