From 70df04687e96df91ab6b37087f8eb383d3861e30 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Fri, 28 Jun 2024 06:18:59 -0700 Subject: [PATCH] remove forwardRef creation for React 19 Summary: Adds a `reactRuntimeTarget` option (defaulting to `'18'`) that when set to '19' no longer adds the `forwardRef` wrapper as React 19 treats refs on function components as regular props. Reviewed By: pieterv Differential Revision: D59131404 fbshipit-source-id: 891e523d0cd0e4d3c374ec4b40d2faee6c83a456 --- .../js/hermes-parser/__test_utils__/parse.js | 13 ++++- .../__tests__/ComponentDeclaration-test.js | 15 ++++++ .../js/hermes-parser/src/ParserOptions.js | 2 + .../src/estree/StripComponentSyntax.js | 50 ++++++++++++------- 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/tools/hermes-parser/js/hermes-parser/__test_utils__/parse.js b/tools/hermes-parser/js/hermes-parser/__test_utils__/parse.js index 72423286d90..ee284827166 100644 --- a/tools/hermes-parser/js/hermes-parser/__test_utils__/parse.js +++ b/tools/hermes-parser/js/hermes-parser/__test_utils__/parse.js @@ -97,8 +97,14 @@ export function printForSnapshotESTree(code: string): Promise { export function parseForSnapshotESTree(code: string): mixed { return parseForSnapshot(code); } -export function printForSnapshotBabel(code: string): Promise { - return printForSnapshot(code, {babel: true}); +export function printForSnapshotBabel( + code: string, + options?: {reactRuntimeTarget?: ParserOptions['reactRuntimeTarget']}, +): Promise { + return printForSnapshot(code, { + babel: true, + reactRuntimeTarget: options?.reactRuntimeTarget, + }); } export function parseForSnapshotBabel(code: string): mixed { return parseForSnapshot(code, {babel: true}); @@ -109,14 +115,17 @@ export async function printForSnapshot( { babel, enableExperimentalComponentSyntax, + reactRuntimeTarget, }: { babel?: boolean, enableExperimentalComponentSyntax?: boolean, + reactRuntimeTarget?: ParserOptions['reactRuntimeTarget'], } = {}, ): Promise { const parseOpts = { enableExperimentalComponentSyntax: enableExperimentalComponentSyntax ?? true, + reactRuntimeTarget, }; if (babel === true) { const ast = parse(source, { diff --git a/tools/hermes-parser/js/hermes-parser/__tests__/ComponentDeclaration-test.js b/tools/hermes-parser/js/hermes-parser/__tests__/ComponentDeclaration-test.js index 3ac23ca99e7..5508a931d28 100644 --- a/tools/hermes-parser/js/hermes-parser/__tests__/ComponentDeclaration-test.js +++ b/tools/hermes-parser/js/hermes-parser/__tests__/ComponentDeclaration-test.js @@ -501,6 +501,21 @@ switch (thing) { foo: string }>, ref: Ref): React.Node {} + callSomething(Foo); + }" + `); + expect(await printForSnapshotBabel(code, {reactRuntimeTarget: '19'})) + .toMatchInlineSnapshot(` + "switch (thing) { + case 1: + function Foo({ + foo, + ref + }: $ReadOnly<{ + foo: string, + ref: Ref, + }>): React.Node {} + callSomething(Foo); }" `); diff --git a/tools/hermes-parser/js/hermes-parser/src/ParserOptions.js b/tools/hermes-parser/js/hermes-parser/src/ParserOptions.js index da0271334a4..16e98ce8cb4 100644 --- a/tools/hermes-parser/js/hermes-parser/src/ParserOptions.js +++ b/tools/hermes-parser/js/hermes-parser/src/ParserOptions.js @@ -13,6 +13,7 @@ export type ParserOptions = { babel?: boolean, flow?: 'all' | 'detect', enableExperimentalComponentSyntax?: boolean, + reactRuntimeTarget?: '18' | '19', sourceFilename?: string, sourceType?: 'module' | 'script' | 'unambiguous', tokens?: boolean, @@ -23,6 +24,7 @@ export const ParserOptionsKeys: $ReadOnlySet<$Keys> = new Set([ 'babel', 'flow', 'enableExperimentalComponentSyntax', + 'reactRuntimeTarget', 'sourceFilename', 'sourceType', 'tokens', diff --git a/tools/hermes-parser/js/hermes-parser/src/estree/StripComponentSyntax.js b/tools/hermes-parser/js/hermes-parser/src/estree/StripComponentSyntax.js index ca86940f911..d42432f9ad2 100644 --- a/tools/hermes-parser/js/hermes-parser/src/estree/StripComponentSyntax.js +++ b/tools/hermes-parser/js/hermes-parser/src/estree/StripComponentSyntax.js @@ -174,6 +174,7 @@ function createPropsTypeAnnotation( function mapComponentParameters( params: $ReadOnlyArray, + options: ParserOptions, ): $ReadOnly<{ props: ?(ObjectPattern | Identifier), ref: ?(BindingName | AssignmentPattern), @@ -195,19 +196,22 @@ function mapComponentParameters( }; } - // Filter out any ref param and capture it's details. + // Filter out any ref param and capture it's details when targeting React 18. + // React 19+ treats ref as a regular prop for function components. let refParam = null; - const paramsWithoutRef = params.filter(param => { - if ( - param.type === 'ComponentParameter' && - getComponentParameterName(param.name) === 'ref' - ) { - refParam = param; - return false; - } - - return true; - }); + const paramsWithoutRef = + (options.reactRuntimeTarget ?? '18') === '18' + ? params.filter(param => { + if ( + param.type === 'ComponentParameter' && + getComponentParameterName(param.name) === 'ref' + ) { + refParam = param; + return false; + } + return true; + }) + : params; const [propTypes, spread] = paramsWithoutRef.reduce< [Array, ?ObjectTypeSpreadProperty], @@ -522,7 +526,10 @@ function createForwardRefWrapper( }; } -function mapComponentDeclaration(node: ComponentDeclaration): { +function mapComponentDeclaration( + node: ComponentDeclaration, + options: ParserOptions, +): { comp: FunctionDeclaration, forwardRefDetails: ?ForwardRefDetails, } { @@ -563,7 +570,7 @@ function mapComponentDeclaration(node: ComponentDeclaration): { ...createRendersTypeLoc(), }; - const {props, ref} = mapComponentParameters(node.params); + const {props, ref} = mapComponentParameters(node.params, options); let forwardRefDetails: ?ForwardRefDetails = null; @@ -685,9 +692,10 @@ function scanForFirstComponentReference( function mapComponentDeclarationIntoList( node: ComponentDeclaration, newBody: Array, + options: ParserOptions, insertExport?: (Identifier | FunctionDeclaration) => ModuleDeclaration, ) { - const {comp, forwardRefDetails} = mapComponentDeclaration(node); + const {comp, forwardRefDetails} = mapComponentDeclaration(node, options); if (forwardRefDetails != null) { // Scan for references to our component. const referencePos = scanForFirstComponentReference( @@ -715,12 +723,13 @@ function mapComponentDeclarationIntoList( function mapStatementList( stmts: $ReadOnlyArray, + options: ParserOptions, ) { const newBody: Array = []; for (const node of stmts) { switch (node.type) { case 'ComponentDeclaration': { - mapComponentDeclarationIntoList(node, newBody); + mapComponentDeclarationIntoList(node, newBody, options); break; } case 'HookDeclaration': { @@ -733,6 +742,7 @@ function mapStatementList( mapComponentDeclarationIntoList( node.declaration, newBody, + options, componentOrRef => { switch (componentOrRef.type) { case 'FunctionDeclaration': { @@ -781,6 +791,7 @@ function mapStatementList( mapComponentDeclarationIntoList( node.declaration, newBody, + options, componentOrRef => nodeWith(node, {declaration: componentOrRef}), ); break; @@ -806,7 +817,7 @@ function mapStatementList( export function transformProgram( program: Program, - _options: ParserOptions, + options: ParserOptions, ): Program { return SimpleTransform.transformProgram(program, { transform(node: ESNode) { @@ -819,13 +830,14 @@ export function transformProgram( } case 'Program': case 'BlockStatement': { - return nodeWith(node, {body: mapStatementList(node.body)}); + return nodeWith(node, {body: mapStatementList(node.body, options)}); } case 'SwitchCase': { + const consequent = mapStatementList(node.consequent, options); return nodeWith(node, { /* $FlowExpectedError[incompatible-call] We know `mapStatementList` will not return `ModuleDeclaration` nodes if it is not passed any */ - consequent: mapStatementList(node.consequent), + consequent, }); } case 'ComponentDeclaration': {