From 0f3ba92fec71ebd7b2e5f773f9963a9a5a945225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Thu, 12 May 2022 11:12:31 +0200 Subject: [PATCH 1/2] Support 'memo(forwardRef(...' --- .../defaultPropsHandler-test.ts.snap | 11 ++++++++++ .../componentDocblockHandler-test.ts | 16 ++++++++++++++ .../__tests__/defaultPropsHandler-test.ts | 13 +++++++++++ src/handlers/defaultPropsHandler.ts | 22 ++++++++++++++----- 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap b/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap index f77a409b805..a3b69e44b7c 100644 --- a/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap +++ b/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap @@ -561,6 +561,17 @@ Object { } `; +exports[`defaultPropsHandler forwardRef resolves default props when memo used 1`] = ` +Object { + "foo": Object { + "defaultValue": Object { + "computed": false, + "value": "'bar'", + }, + }, +} +`; + exports[`defaultPropsHandler forwardRef resolves defaultProps 1`] = ` Object { "foo": Object { diff --git a/src/handlers/__tests__/componentDocblockHandler-test.ts b/src/handlers/__tests__/componentDocblockHandler-test.ts index 3cee7df4062..d268b035924 100644 --- a/src/handlers/__tests__/componentDocblockHandler-test.ts +++ b/src/handlers/__tests__/componentDocblockHandler-test.ts @@ -320,6 +320,22 @@ describe('componentDocblockHandler', () => { ); }); + describe('inline implementation with memo', () => { + test(` + React.memo(React.forwardRef((props, ref) => {})); + import React from "react";`, src => + beforeLastStatement(src).get('expression')); + + testImports( + ` + export default React.memo(React.forwardRef((props, ref) => {})); + import React from 'react';`, + src => beforeLastStatement(src).get('declaration'), + 'RefComponent', + useDefault, + ); + }); + describe('out of line implementation', () => { test(`let Component = (props, ref) => {}; React.forwardRef(Component); diff --git a/src/handlers/__tests__/defaultPropsHandler-test.ts b/src/handlers/__tests__/defaultPropsHandler-test.ts index f0156aa774d..6504eaf2056 100644 --- a/src/handlers/__tests__/defaultPropsHandler-test.ts +++ b/src/handlers/__tests__/defaultPropsHandler-test.ts @@ -573,6 +573,19 @@ describe('defaultPropsHandler', () => { expect(documentation.descriptors).toMatchSnapshot(); }); + it('resolves default props when memo used', () => { + const src = ` + import React, {memo, forwardRef} from 'react'; + memo(forwardRef(({ foo = 'bar' }, ref) =>
{foo}
)); + `; + defaultPropsHandler( + documentation, + parse(src).get('body', 1, 'expression'), + noopImporter, + ); + expect(documentation.descriptors).toMatchSnapshot(); + }); + it('resolves imported default props in the parameters', () => { const src = ` import baz from 'baz'; diff --git a/src/handlers/defaultPropsHandler.ts b/src/handlers/defaultPropsHandler.ts index 226281100da..f908e08288b 100644 --- a/src/handlers/defaultPropsHandler.ts +++ b/src/handlers/defaultPropsHandler.ts @@ -9,6 +9,7 @@ import isReactForwardRefCall from '../utils/isReactForwardRefCall'; import type Documentation from '../Documentation'; import type { Importer } from '../parse'; import type { NodePath } from 'ast-types/lib/node-path'; +import isReactBuiltinCall from '../utils/isReactBuiltinCall'; function getDefaultValue(path: NodePath, importer: Importer) { let node = path.node; @@ -46,12 +47,21 @@ function getStatelessPropsPath( componentDefinition: NodePath, importer: Importer, ): NodePath { - const value = resolveToValue(componentDefinition, importer); - if (isReactForwardRefCall(value, importer)) { - const inner = resolveToValue(value.get('arguments', 0), importer); - return inner.get('params', 0); - } - return value.get('params', 0); + let actPath = componentDefinition; + let changed = false; + do { + changed = false; + const value = resolveToValue(actPath, importer); + if ( + isReactBuiltinCall(value, 'memo', importer) || + isReactForwardRefCall(value, importer) + ) { + actPath = resolveToValue(actPath.get('arguments', 0), importer); + changed = true; + } + } while (changed); + + return actPath.get('params', 0); } function getDefaultPropsPath( From ee024f64daf8990ed443634f000c0f793528454b Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Mon, 13 Jun 2022 18:17:59 +0000 Subject: [PATCH 2/2] fix: Fix wrong detection of forwardRef in combination with memo --- src/__tests__/__snapshots__/main-test.ts.snap | 16 ++++++++++++++ src/__tests__/fixtures/component_42.js | 3 +++ .../defaultPropsHandler-test.ts.snap | 11 ---------- .../__tests__/defaultPropsHandler-test.ts | 13 ------------ src/handlers/defaultPropsHandler.ts | 21 ++++++------------- .../__tests__/isReactForwardRefCall-test.ts | 8 +++++++ src/utils/isReactBuiltinCall.ts | 13 ++++++------ src/utils/isReactCreateClassCall.ts | 3 +-- 8 files changed, 41 insertions(+), 47 deletions(-) create mode 100644 src/__tests__/fixtures/component_42.js diff --git a/src/__tests__/__snapshots__/main-test.ts.snap b/src/__tests__/__snapshots__/main-test.ts.snap index 99041fe6d4f..fc6ededf638 100644 --- a/src/__tests__/__snapshots__/main-test.ts.snap +++ b/src/__tests__/__snapshots__/main-test.ts.snap @@ -1801,6 +1801,22 @@ Object { } `; +exports[`main fixtures processes component "component_42.js" without errors 1`] = ` +Object { + "description": "", + "methods": Array [], + "props": Object { + "foo": Object { + "defaultValue": Object { + "computed": false, + "value": "'bar'", + }, + "required": false, + }, + }, +} +`; + exports[`main fixtures processes component "flow-export-type.js" without errors 1`] = ` Object { "description": "This is a Flow class component", diff --git a/src/__tests__/fixtures/component_42.js b/src/__tests__/fixtures/component_42.js new file mode 100644 index 00000000000..6e40462f356 --- /dev/null +++ b/src/__tests__/fixtures/component_42.js @@ -0,0 +1,3 @@ +import React, {memo, forwardRef} from 'react'; + +export default memo(forwardRef(({ foo = 'bar' }, ref) =>
{foo}
)); \ No newline at end of file diff --git a/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap b/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap index a3b69e44b7c..f77a409b805 100644 --- a/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap +++ b/src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.ts.snap @@ -561,17 +561,6 @@ Object { } `; -exports[`defaultPropsHandler forwardRef resolves default props when memo used 1`] = ` -Object { - "foo": Object { - "defaultValue": Object { - "computed": false, - "value": "'bar'", - }, - }, -} -`; - exports[`defaultPropsHandler forwardRef resolves defaultProps 1`] = ` Object { "foo": Object { diff --git a/src/handlers/__tests__/defaultPropsHandler-test.ts b/src/handlers/__tests__/defaultPropsHandler-test.ts index 6504eaf2056..f0156aa774d 100644 --- a/src/handlers/__tests__/defaultPropsHandler-test.ts +++ b/src/handlers/__tests__/defaultPropsHandler-test.ts @@ -573,19 +573,6 @@ describe('defaultPropsHandler', () => { expect(documentation.descriptors).toMatchSnapshot(); }); - it('resolves default props when memo used', () => { - const src = ` - import React, {memo, forwardRef} from 'react'; - memo(forwardRef(({ foo = 'bar' }, ref) =>
{foo}
)); - `; - defaultPropsHandler( - documentation, - parse(src).get('body', 1, 'expression'), - noopImporter, - ); - expect(documentation.descriptors).toMatchSnapshot(); - }); - it('resolves imported default props in the parameters', () => { const src = ` import baz from 'baz'; diff --git a/src/handlers/defaultPropsHandler.ts b/src/handlers/defaultPropsHandler.ts index f908e08288b..d5496bcd952 100644 --- a/src/handlers/defaultPropsHandler.ts +++ b/src/handlers/defaultPropsHandler.ts @@ -9,7 +9,6 @@ import isReactForwardRefCall from '../utils/isReactForwardRefCall'; import type Documentation from '../Documentation'; import type { Importer } from '../parse'; import type { NodePath } from 'ast-types/lib/node-path'; -import isReactBuiltinCall from '../utils/isReactBuiltinCall'; function getDefaultValue(path: NodePath, importer: Importer) { let node = path.node; @@ -47,21 +46,13 @@ function getStatelessPropsPath( componentDefinition: NodePath, importer: Importer, ): NodePath { - let actPath = componentDefinition; - let changed = false; - do { - changed = false; - const value = resolveToValue(actPath, importer); - if ( - isReactBuiltinCall(value, 'memo', importer) || - isReactForwardRefCall(value, importer) - ) { - actPath = resolveToValue(actPath.get('arguments', 0), importer); - changed = true; - } - } while (changed); + let value = resolveToValue(componentDefinition, importer); + + if (isReactForwardRefCall(value, importer)) { + value = resolveToValue(value.get('arguments', 0), importer); + } - return actPath.get('params', 0); + return value.get('params', 0); } function getDefaultPropsPath( diff --git a/src/utils/__tests__/isReactForwardRefCall-test.ts b/src/utils/__tests__/isReactForwardRefCall-test.ts index a973f70d4cb..7cb34525424 100644 --- a/src/utils/__tests__/isReactForwardRefCall-test.ts +++ b/src/utils/__tests__/isReactForwardRefCall-test.ts @@ -77,6 +77,14 @@ describe('isReactForwardRefCall', () => { expect(isReactForwardRefCall(def, noopImporter)).toBe(true); }); + it('does not accept forwardRef if not outer call', () => { + const def = expressionLast(` + import { forwardRef, memo } from "react"; + memo(forwardRef({})); + `); + expect(isReactForwardRefCall(def, noopImporter)).toBe(false); + }); + it('accepts forwardRef called on imported aliased value', () => { const def = expressionLast(` import { forwardRef as foo } from "react"; diff --git a/src/utils/isReactBuiltinCall.ts b/src/utils/isReactBuiltinCall.ts index c96316ec723..e6adee99213 100644 --- a/src/utils/isReactBuiltinCall.ts +++ b/src/utils/isReactBuiltinCall.ts @@ -31,10 +31,8 @@ export default function isReactBuiltinCall( // Check if this is a destructuring assignment // const { x } = require('react') - if (isDestructuringAssignment(value, name)) { - const module = resolveToModule(value, importer); - return Boolean(module && isReactModuleName(module)); - } else if ( + if ( + isDestructuringAssignment(value, name) || // `require('react').createElement` (t.MemberExpression.check(value.node) && t.Identifier.check(value.get('property').node) && @@ -43,11 +41,14 @@ export default function isReactBuiltinCall( (t.ImportDeclaration.check(value.node) && value.node.specifiers && value.node.specifiers.some( - // @ts-ignore - specifier => specifier.imported && specifier.imported.name === name, + specifier => + // @ts-ignore + specifier.imported?.name === name && + specifier.local?.name === path.node.callee.name, )) ) { const module = resolveToModule(value, importer); + return Boolean(module && isReactModuleName(module)); } } diff --git a/src/utils/isReactCreateClassCall.ts b/src/utils/isReactCreateClassCall.ts index b6d3aa11e97..38e5a4cb574 100644 --- a/src/utils/isReactCreateClassCall.ts +++ b/src/utils/isReactCreateClassCall.ts @@ -1,5 +1,4 @@ import { namedTypes as t } from 'ast-types'; -import match from './match'; import resolveToModule from './resolveToModule'; import isReactBuiltinCall from './isReactBuiltinCall'; import type { Importer } from '../parse'; @@ -20,7 +19,7 @@ function isReactCreateClassCallModular( path = path.get('expression'); } - if (!match(path.node, { type: 'CallExpression' })) { + if (!t.CallExpression.check(path.node)) { return false; } const module = resolveToModule(path, importer);