From ea9cbebef13de11d591f175438e59b48dbb67025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Mon, 13 Jun 2022 20:25:58 +0200 Subject: [PATCH] fix: Fix wrong detection of forwardRef in combination with memo (#592) Release-As: 6.0.0-alpha.3 Co-authored-by: Daniel Tschinder <231804+danez@users.noreply.github.com> --- src/__tests__/__snapshots__/main-test.ts.snap | 16 ++++++++++++++++ src/__tests__/fixtures/component_42.js | 3 +++ .../__tests__/componentDocblockHandler-test.ts | 16 ++++++++++++++++ src/handlers/defaultPropsHandler.ts | 7 ++++--- .../__tests__/isReactForwardRefCall-test.ts | 8 ++++++++ src/utils/isReactBuiltinCall.ts | 13 +++++++------ src/utils/isReactCreateClassCall.ts | 3 +-- 7 files changed, 55 insertions(+), 11 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 01fdcf44668..2ad0d868306 100644 --- a/src/__tests__/__snapshots__/main-test.ts.snap +++ b/src/__tests__/__snapshots__/main-test.ts.snap @@ -1819,6 +1819,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__/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/defaultPropsHandler.ts b/src/handlers/defaultPropsHandler.ts index 226281100da..d5496bcd952 100644 --- a/src/handlers/defaultPropsHandler.ts +++ b/src/handlers/defaultPropsHandler.ts @@ -46,11 +46,12 @@ function getStatelessPropsPath( componentDefinition: NodePath, importer: Importer, ): NodePath { - const value = resolveToValue(componentDefinition, importer); + let value = resolveToValue(componentDefinition, importer); + if (isReactForwardRefCall(value, importer)) { - const inner = resolveToValue(value.get('arguments', 0), importer); - return inner.get('params', 0); + value = resolveToValue(value.get('arguments', 0), importer); } + return value.get('params', 0); } 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);