From 58e3b1a9c7cb88bd7a23c88a2188c2008bf2b10b Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Sat, 19 Sep 2026 05:11:05 +0000 Subject: [PATCH 01/10] fix: detect useCallback methods in imperative handles --- ...omponentMethodsHandler-useCallback-test.ts | 33 +++++++++++++++++++ .../src/handlers/componentMethodsHandler.ts | 14 ++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts 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..635ab9c86fa --- /dev/null +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -0,0 +1,33 @@ +import { parse } from '../../../tests/utils'; +import componentMethodsHandler from '../componentMethodsHandler.js'; +import DocumentationBuilder from '../../Documentation'; +import type DocumentationMock from '../../__mocks__/Documentation'; +import type { FunctionDeclaration } 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; + }); + + test('extracts a method wrapped with useCallback', () => { + const definition = parse.statementLast(` + import { useCallback, useImperativeHandle } from 'react'; + function Component() { + const method = useCallback((argument: string): number => 1, []); + useImperativeHandle(ref, () => ({ method })); + return
; + } + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]?.name).toBe('method'); + }); +}); diff --git a/packages/react-docgen/src/handlers/componentMethodsHandler.ts b/packages/react-docgen/src/handlers/componentMethodsHandler.ts index 2bbcd571473..06f4c54c589 100644 --- a/packages/react-docgen/src/handlers/componentMethodsHandler.ts +++ b/packages/react-docgen/src/handlers/componentMethodsHandler.ts @@ -41,8 +41,18 @@ function isMethod(path: NodePath): path is MethodNodePath { (path.isClassProperty() || path.isObjectProperty()) ) { const value = resolveToValue(path.get('value') as NodePath); - - isProbablyMethod = value.isFunction(); + const callback = + value.isCallExpression() && isReactBuiltinCall(value, 'useCallback') + ? value.get('arguments')[0] + : undefined; + + isProbablyMethod = + value.isFunction() || + Boolean( + callback && + !Array.isArray(callback) && + resolveToValue(callback).isFunction(), + ); } return isProbablyMethod && !isReactComponentMethod(path); From 93a61a7f459dbd153b162605010128eccbc27f3f Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Sat, 19 Sep 2026 05:47:56 +0000 Subject: [PATCH 02/10] test: cover imperative handle callback references --- ...omponentMethodsHandler-useCallback-test.ts | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts index 635ab9c86fa..3c8202b355d 100644 --- a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -15,19 +15,45 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { DocumentationMock; }); - test('extracts a method wrapped with useCallback', () => { - const definition = parse.statementLast(` - import { useCallback, useImperativeHandle } from 'react'; - function Component() { - const method = useCallback((argument: string): number => 1, []); - useImperativeHandle(ref, () => ({ method })); - return
; - } - `); + 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', + }, + ])( + '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); + componentMethodsHandler(documentation, definition); - expect(documentation.methods).toHaveLength(1); - expect(documentation.methods[0]?.name).toBe('method'); - }); + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]?.name).toBe('method'); + }, + ); }); From 066e074dea05dc8559838518230ff301c62a7a73 Mon Sep 17 00:00:00 2001 From: askalf Date: Sat, 19 Sep 2026 03:16:02 -0400 Subject: [PATCH 03/10] fix: document the function a useCallback method wraps The handler recognised a useCallback-wrapped property as a method, but the documentation was built from the call expression, so the method came out with no parameters and no return type. resolveToMethodFunction now resolves a value to the function it documents, unwrapping a React useCallback call, and both the handler's method test and the documentation builder use it. The tests assert the wrapped function's signature and cover a local function named useCallback, a call with no arguments and a non-function argument. --- ...omponentMethodsHandler-useCallback-test.ts | 47 ++++++++++++++++++- .../src/handlers/componentMethodsHandler.ts | 17 ++----- .../src/utils/getMethodDocumentation.ts | 37 ++++++++++++--- 3 files changed, 80 insertions(+), 21 deletions(-) diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts index 3c8202b355d..5fe5fdf7a32 100644 --- a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -52,8 +52,53 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { 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]?.name).toBe('method'); + expect(documentation.methods[0]).toMatchObject({ + name: 'method', + params: [ + { name: 'argument', optional: false, type: { name: 'string' } }, + ], + returns: { type: { name: 'number' } }, + }); }, ); + + test.each([ + { + name: 'a local function named useCallback', + imports: "import { useImperativeHandle } from 'react';", + setup: + 'function useCallback(fn: unknown, deps: unknown[]) { return fn; }', + value: 'useCallback((argument: string): number => 1, [])', + }, + { + name: 'a useCallback call with no arguments', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: '', + value: 'useCallback()', + }, + { + name: 'a useCallback call whose first argument is not a function', + imports: "import { useCallback, useImperativeHandle } from 'react';", + setup: '', + value: 'useCallback(42, [])', + }, + ])('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 06f4c54c589..93e4fd1cb49 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, { + resolveToMethodFunction, +} from '../utils/getMethodDocumentation.js'; import isReactComponentClass from '../utils/isReactComponentClass.js'; import isReactComponentMethod from '../utils/isReactComponentMethod.js'; import type Documentation from '../Documentation.js'; @@ -40,19 +42,8 @@ function isMethod(path: NodePath): path is MethodNodePath { !isProbablyMethod && (path.isClassProperty() || path.isObjectProperty()) ) { - const value = resolveToValue(path.get('value') as NodePath); - const callback = - value.isCallExpression() && isReactBuiltinCall(value, 'useCallback') - ? value.get('arguments')[0] - : undefined; - isProbablyMethod = - value.isFunction() || - Boolean( - callback && - !Array.isArray(callback) && - resolveToValue(callback).isFunction(), - ); + resolveToMethodFunction(path.get('value') as NodePath) !== null; } return isProbablyMethod && !isReactComponentMethod(path); diff --git a/packages/react-docgen/src/utils/getMethodDocumentation.ts b/packages/react-docgen/src/utils/getMethodDocumentation.ts index 3dc0162ea53..5d46cdf2abb 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,6 +33,34 @@ export type MethodNodePath = | NodePath | NodePath; +/** + * Resolves a value to the function it documents: the value itself, or the + * function a React `useCallback` call wraps. Null when it is neither. + */ +export function resolveToMethodFunction( + path: NodePath, +): NodePath | null { + const value = resolveToValue(path); + + if (value.isFunction()) { + return value; + } + + 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, ): NodePath | null { @@ -43,13 +72,7 @@ function getMethodFunctionExpression( ? methodPath.get('right') : (methodPath.get('value') as NodePath); - const functionExpression = resolveToValue(potentialFunctionExpression); - - if (functionExpression.isFunction()) { - return functionExpression; - } - - return null; + return resolveToMethodFunction(potentialFunctionExpression); } function getMethodParamOptional( From b002e5066dd6b3afdd1f8957333c14aae5b5b404 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Sat, 19 Sep 2026 19:48:43 +0000 Subject: [PATCH 04/10] test: cover every surface that documents a useCallback method The handler's unwrapping now runs for every method node path, so add the surfaces it reaches beyond the imperative handle identifier: a callback written inline in the handle object, an ObjectExpression component, a statics object, a class property and a Component.foo assignment, plus the docblock and the async/generator modifiers of the wrapped function, a renamed useCallback import and a handle that precedes the declaration. The controls pin the guards the unwrapping keeps: a local useCallback, a missing or non-function argument, a spread argument, a nested call and useMemo. --- ...omponentMethodsHandler-useCallback-test.ts | 213 +++++++++++++++++- ...getMethodDocumentation-useCallback-test.ts | 46 ++++ 2 files changed, 251 insertions(+), 8 deletions(-) create mode 100644 packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts index 5fe5fdf7a32..4ad7b66cdd2 100644 --- a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -2,7 +2,11 @@ import { parse } from '../../../tests/utils'; import componentMethodsHandler from '../componentMethodsHandler.js'; import DocumentationBuilder from '../../Documentation'; import type DocumentationMock from '../../__mocks__/Documentation'; -import type { FunctionDeclaration } from '@babel/types'; +import type { + ClassDeclaration, + FunctionDeclaration, + ObjectExpression, +} from '@babel/types'; import { beforeEach, describe, expect, test, vi } from 'vitest'; vi.mock('../../Documentation.js'); @@ -15,6 +19,12 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { DocumentationMock; }); + const wrappedSignature = { + name: 'method', + params: [{ name: 'argument', optional: false, type: { name: 'string' } }], + returns: { type: { name: 'number' } }, + }; + test.each([ { name: 'a directly-declared callback', @@ -37,6 +47,14 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { 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 }) => { @@ -54,37 +72,216 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { // 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', - params: [ - { name: 'argument', optional: false, type: { name: 'string' } }, - ], - returns: { type: { name: 'number' } }, + modifiers, }); }, ); + test('extracts a callback method on an ObjectExpression component', () => { + const definition = parse.expressionLast(` + import { useCallback } from 'react'; + ({ + method: useCallback((argument: string): number => 1, []), + }) + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }); + + test('extracts a callback method in a statics object', () => { + const definition = parse.expressionLast(` + import { useCallback } from 'react'; + ({ + statics: { + method: useCallback((argument: string): number => 1, []), + }, + }) + `); + + componentMethodsHandler(documentation, definition); + + expect(documentation.methods).toHaveLength(1); + expect(documentation.methods[0]).toMatchObject({ + ...wrappedSignature, + modifiers: ['static'], + }); + }); + + test('extracts a callback class property', () => { + 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(1); + expect(documentation.methods[0]).toMatchObject(wrappedSignature); + }); + + test('documents a plain function class property (control)', () => { + // Controls for the class-property surface itself: it documented plain + // function values before this change, so the callback case above fails + // on the unwrapping, not on the surface. + 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([ { - name: 'a local function named useCallback', + // 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, [])', }, { - name: 'a useCallback call with no arguments', + // 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()', }, { - name: 'a useCallback call whose first argument is not a function', + // 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} diff --git a/packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts b/packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts new file mode 100644 index 00000000000..0f4a2d8eb08 --- /dev/null +++ b/packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts @@ -0,0 +1,46 @@ +import type { NodePath } from '@babel/traverse'; +import type { AssignmentExpression, ExpressionStatement } from '@babel/types'; +import { parse } from '../../../tests/utils'; +import getMethodDocumentation from '../getMethodDocumentation.js'; +import { describe, expect, test } from 'vitest'; + +describe('getMethodDocumentation useCallback', () => { + test('documents the function a useCallback assignment wraps', () => { + const method = parse + .statementLast( + `import { useCallback } from 'react'; + const Foo = () => {} + Foo.foo = useCallback((bar: number): number => bar, []) + `, + ) + .get('expression') as NodePath; + + expect(getMethodDocumentation(method)).toEqual({ + name: 'foo', + docblock: null, + modifiers: ['static'], + returns: { type: { name: 'number' } }, + params: [{ name: 'bar', optional: false, type: { name: 'number' } }], + }); + }); + + test('documents a plain function assignment (control)', () => { + // Controls for the assignment surface: it documented plain function + // values before this change, so the case above fails on the unwrapping. + const method = parse + .statementLast( + `const Foo = () => {} + Foo.foo = (bar: number): number => bar + `, + ) + .get('expression') as NodePath; + + expect(getMethodDocumentation(method)).toEqual({ + name: 'foo', + docblock: null, + modifiers: ['static'], + returns: { type: { name: 'number' } }, + params: [{ name: 'bar', optional: false, type: { name: 'number' } }], + }); + }); +}); From 59f7f195b6ce434ab0ec5fc8ba6db11befcdd61e Mon Sep 17 00:00:00 2001 From: askalf Date: Sat, 19 Sep 2026 16:01:46 -0400 Subject: [PATCH 05/10] test: pin the zero-parameter, unannotated useCallback method Every positive callback fixture carried a typed parameter and a return annotation, so the empty-signature path the boundary ledger claims was never exercised. Admit useCallback(() => {}, []) through useImperativeHandle and assert an empty params list and a null return. --- ...omponentMethodsHandler-useCallback-test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts index 4ad7b66cdd2..7eb84333296 100644 --- a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -167,6 +167,26 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { }, ); + 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('extracts a callback method on an ObjectExpression component', () => { const definition = parse.expressionLast(` import { useCallback } from 'react'; From a9605a453f76d1d8ade292b0f9e45c81e2442ba2 Mon Sep 17 00:00:00 2001 From: askalf Date: Sat, 19 Sep 2026 21:31:03 -0400 Subject: [PATCH 06/10] chore: add changeset for the useCallback method fix --- .changeset/quiet-hooks-listen.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/quiet-hooks-listen.md 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` From c97359c0900ee1c563248c969fcd6eb355b368cd Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:09:01 +0000 Subject: [PATCH 07/10] test: run the reported component through the parser The handler tests build a component definition and call the handler directly, so none of them exercise the resolver path the issue reports: a memo(forwardRef) component whose method is exposed as a shorthand property. Parse that component verbatim through the public parse() and assert the documented method, with the same component without useCallback as a control for memo/forwardRef resolution. --- .../imperativeHandleCallbacks-test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 packages/react-docgen/src/__tests__/imperativeHandleCallbacks-test.ts 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] }); + }); +}); From f5d79394de9abe70d5b2cb7be23b29f3b2005569 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:09:01 +0000 Subject: [PATCH 08/10] fix: keep the useCallback unwrapping on the imperative handle path Documenting the function a useCallback call wraps changed every method surface, including class fields and statics objects where hooks are not valid React usage. Pass the imperative handle down to getMethodDocumentation and unwrap only there; the other surfaces keep documenting plain function values only. The surface tests become controls: they assert that an object, statics and class-property useCallback stays undocumented, next to a plain function on the same surface that still is. --- ...omponentMethodsHandler-useCallback-test.ts | 46 ++++++++++----- .../src/handlers/componentMethodsHandler.ts | 32 ++++++++-- .../src/utils/getMethodDocumentation.ts | 58 +++++++++++++------ 3 files changed, 98 insertions(+), 38 deletions(-) diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts index 7eb84333296..e361f91db3e 100644 --- a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -187,7 +187,29 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { }); }); - test('extracts a callback method on an ObjectExpression component', () => { + 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); + }); + + // 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'; ({ @@ -197,11 +219,10 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { componentMethodsHandler(documentation, definition); - expect(documentation.methods).toHaveLength(1); - expect(documentation.methods[0]).toMatchObject(wrappedSignature); + expect(documentation.methods).toHaveLength(0); }); - test('extracts a callback method in a statics object', () => { + test('does not document a callback method in a statics object (control)', () => { const definition = parse.expressionLast(` import { useCallback } from 'react'; ({ @@ -213,14 +234,10 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { componentMethodsHandler(documentation, definition); - expect(documentation.methods).toHaveLength(1); - expect(documentation.methods[0]).toMatchObject({ - ...wrappedSignature, - modifiers: ['static'], - }); + expect(documentation.methods).toHaveLength(0); }); - test('extracts a callback class property', () => { + test('does not document a callback class property (control)', () => { const definition = parse.statementLast(` import React, { useCallback } from 'react'; class Test extends React.Component { @@ -231,14 +248,13 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { componentMethodsHandler(documentation, definition); - expect(documentation.methods).toHaveLength(1); - expect(documentation.methods[0]).toMatchObject(wrappedSignature); + expect(documentation.methods).toHaveLength(0); }); test('documents a plain function class property (control)', () => { - // Controls for the class-property surface itself: it documented plain - // function values before this change, so the callback case above fails - // on the unwrapping, not on the surface. + // 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 { diff --git a/packages/react-docgen/src/handlers/componentMethodsHandler.ts b/packages/react-docgen/src/handlers/componentMethodsHandler.ts index 93e4fd1cb49..93b9965002f 100644 --- a/packages/react-docgen/src/handlers/componentMethodsHandler.ts +++ b/packages/react-docgen/src/handlers/componentMethodsHandler.ts @@ -1,7 +1,7 @@ import getMemberValuePath from '../utils/getMemberValuePath.js'; import type { MethodNodePath } from '../utils/getMethodDocumentation.js'; import getMethodDocumentation, { - resolveToMethodFunction, + resolveToUseCallbackFunction, } from '../utils/getMethodDocumentation.js'; import isReactComponentClass from '../utils/isReactComponentClass.js'; import isReactComponentMethod from '../utils/isReactComponentMethod.js'; @@ -42,13 +42,30 @@ function isMethod(path: NodePath): path is MethodNodePath { !isProbablyMethod && (path.isClassProperty() || path.isObjectProperty()) ) { - isProbablyMethod = - resolveToMethodFunction(path.get('value') as NodePath) !== null; + const value = resolveToValue(path.get('value') as NodePath); + + isProbablyMethod = value.isFunction(); } 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; @@ -80,6 +97,7 @@ const explodedVisitors = visitors.explode({ interface MethodDefinition { path: MethodNodePath; + isImperativeHandle?: boolean; isStatic?: boolean; } @@ -122,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); } }); @@ -167,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( @@ -268,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 5d46cdf2abb..119dbc3f8ce 100644 --- a/packages/react-docgen/src/utils/getMethodDocumentation.ts +++ b/packages/react-docgen/src/utils/getMethodDocumentation.ts @@ -33,19 +33,24 @@ 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; +} + /** - * Resolves a value to the function it documents: the value itself, or the - * function a React `useCallback` call wraps. Null when it is neither. + * 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 resolveToMethodFunction( +export function resolveToUseCallbackFunction( path: NodePath, ): NodePath | null { const value = resolveToValue(path); - if (value.isFunction()) { - return value; - } - if (value.isCallExpression() && isReactBuiltinCall(value, 'useCallback')) { const callback = value.get('arguments')[0]; @@ -63,6 +68,7 @@ export function resolveToMethodFunction( function getMethodFunctionExpression( methodPath: MethodNodePath, + options: MethodOptions, ): NodePath | null { if (methodPath.isClassMethod() || methodPath.isObjectMethod()) { return methodPath; @@ -72,7 +78,19 @@ function getMethodFunctionExpression( ? methodPath.get('right') : (methodPath.get('value') as NodePath); - return resolveToMethodFunction(potentialFunctionExpression); + const functionExpression = resolveToValue(potentialFunctionExpression); + + if (functionExpression.isFunction()) { + 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; } function getMethodParamOptional( @@ -91,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. @@ -129,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')); @@ -151,7 +175,7 @@ function getMethodReturnDoc(methodPath: MethodNodePath): MethodReturn | null { function getMethodModifiers( methodPath: MethodNodePath, - options: { isStatic?: boolean }, + options: MethodOptions, ): MethodModifier[] { if (methodPath.isAssignmentExpression()) { return ['static']; @@ -169,7 +193,7 @@ function getMethodModifiers( modifiers.push('static'); } - const functionExpression = getMethodFunctionExpression(methodPath); + const functionExpression = getMethodFunctionExpression(methodPath, options); if (functionExpression) { if ( @@ -253,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' || @@ -270,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), }; } From e9a51c50dec72109be96c12137d529b2b1f0bfb4 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:09:01 +0000 Subject: [PATCH 09/10] test: drop the out-of-scope assignment case findAssignedMethods only collects assignments whose right-hand side resolves to a function, so Component.foo = useCallback(...) never reaches getMethodDocumentation through the handler. Remove the unit test that implied the assignment surface supported it. --- ...getMethodDocumentation-useCallback-test.ts | 46 ------------------- 1 file changed, 46 deletions(-) delete mode 100644 packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts diff --git a/packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts b/packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts deleted file mode 100644 index 0f4a2d8eb08..00000000000 --- a/packages/react-docgen/src/utils/__tests__/getMethodDocumentation-useCallback-test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { NodePath } from '@babel/traverse'; -import type { AssignmentExpression, ExpressionStatement } from '@babel/types'; -import { parse } from '../../../tests/utils'; -import getMethodDocumentation from '../getMethodDocumentation.js'; -import { describe, expect, test } from 'vitest'; - -describe('getMethodDocumentation useCallback', () => { - test('documents the function a useCallback assignment wraps', () => { - const method = parse - .statementLast( - `import { useCallback } from 'react'; - const Foo = () => {} - Foo.foo = useCallback((bar: number): number => bar, []) - `, - ) - .get('expression') as NodePath; - - expect(getMethodDocumentation(method)).toEqual({ - name: 'foo', - docblock: null, - modifiers: ['static'], - returns: { type: { name: 'number' } }, - params: [{ name: 'bar', optional: false, type: { name: 'number' } }], - }); - }); - - test('documents a plain function assignment (control)', () => { - // Controls for the assignment surface: it documented plain function - // values before this change, so the case above fails on the unwrapping. - const method = parse - .statementLast( - `const Foo = () => {} - Foo.foo = (bar: number): number => bar - `, - ) - .get('expression') as NodePath; - - expect(getMethodDocumentation(method)).toEqual({ - name: 'foo', - docblock: null, - modifiers: ['static'], - returns: { type: { name: 'number' } }, - params: [{ name: 'bar', optional: false, type: { name: 'number' } }], - }); - }); -}); From 1d45d9854382e5e5c3e547614d2959e6a6e93e55 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:12:14 +0000 Subject: [PATCH 10/10] test: pin the lifecycle filter on an unwrapped callback The imperative handle admission keeps the React lifecycle check, so a handle property named after a lifecycle method stays undocumented even when its value is a useCallback call. Control: it passes with and without the unwrapping. --- .../componentMethodsHandler-useCallback-test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts index e361f91db3e..df518c0fce4 100644 --- a/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts +++ b/packages/react-docgen/src/handlers/__tests__/componentMethodsHandler-useCallback-test.ts @@ -206,6 +206,23 @@ describe('componentMethodsHandler useImperativeHandle callbacks', () => { 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.