Conversation
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.
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.
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.
🦋 Changeset detectedLatest commit: a9605a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for react-docgen canceled.
|
There was a problem hiding this comment.
Thanks for the PR.
The core useImperativeHandle + useCallback support makes sense, but I think the tests and implementation should be tightened before merging. In particular, please cover the exact issue reproducer and either narrow or consistently implement the additional method surfaces.
| ])( | ||
| 'extracts a method wrapped with $name', | ||
| ({ imports, setup, value, imperativeHandle }) => { | ||
| const definition = parse.statementLast<FunctionDeclaration>(` |
There was a problem hiding this comment.
Could we add a regression test that runs the exact shape from #856 through the normal parser/resolver: memo(forwardRef(...)), a top-level useCallback, and shorthand exposure through useImperativeHandle? These tests invoke the handler directly with a plain function, so they do not pin the wrapper-resolution path from the reported bug.
AI disclosure: This comment was added by ChatGPT 5.6 Sol.
|
|
||
| isProbablyMethod = value.isFunction(); | ||
| isProbablyMethod = | ||
| resolveToMethodFunction(path.get('value') as NodePath) !== null; |
There was a problem hiding this comment.
This changes generic object and class-property method detection, although #856 only asks about methods exposed through useImperativeHandle. It also adds support for useCallback in class fields/statics, where Hooks are not valid React usage. Could we keep the unwrapping local to the imperative-handle path unless there is a separate use case for broadening every method surface?
AI disclosure: This comment was added by ChatGPT 5.6 Sol.
| .statementLast<ExpressionStatement>( | ||
| `import { useCallback } from 'react'; | ||
| const Foo = () => {} | ||
| Foo.foo = useCallback((bar: number): number => bar, []) |
There was a problem hiding this comment.
This test suggests that Component.foo = useCallback(...) is supported, but findAssignedMethods still filters assignments with resolveToValue(right).isFunction(), so the public handler never passes this assignment to getMethodDocumentation. Please either update the handler consistently and test it through componentMethodsHandler, or remove this out-of-scope assignment case.
AI disclosure: This comment was added by ChatGPT 5.6 Sol.
2 participants
Fixes #856
Summary
useCallbackwhen documenting component methods.Decisions
Resolution intentionally unwraps one React
useCallbacklayer only; recursively unwrapping or treating other React builtins as methods would broaden behavior beyond this defect. The package suite and formatting checks passed.AI assistance: this bug was found and the fix and tests were drafted with AI tooling in my workflow; the tests and checks above were executed as pasted. I'm responsible for the change and will handle review feedback.