Skip to content

Commit

Permalink
remove forwardRef creation for React 19
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kassens authored and facebook-github-bot committed Jun 28, 2024
1 parent 3034371 commit 70df046
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 21 deletions.
13 changes: 11 additions & 2 deletions tools/hermes-parser/js/hermes-parser/__test_utils__/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,14 @@ export function printForSnapshotESTree(code: string): Promise<string> {
export function parseForSnapshotESTree(code: string): mixed {
return parseForSnapshot(code);
}
export function printForSnapshotBabel(code: string): Promise<string> {
return printForSnapshot(code, {babel: true});
export function printForSnapshotBabel(
code: string,
options?: {reactRuntimeTarget?: ParserOptions['reactRuntimeTarget']},
): Promise<string> {
return printForSnapshot(code, {
babel: true,
reactRuntimeTarget: options?.reactRuntimeTarget,
});
}
export function parseForSnapshotBabel(code: string): mixed {
return parseForSnapshot(code, {babel: true});
Expand All @@ -109,14 +115,17 @@ export async function printForSnapshot(
{
babel,
enableExperimentalComponentSyntax,
reactRuntimeTarget,
}: {
babel?: boolean,
enableExperimentalComponentSyntax?: boolean,
reactRuntimeTarget?: ParserOptions['reactRuntimeTarget'],
} = {},
): Promise<string> {
const parseOpts = {
enableExperimentalComponentSyntax:
enableExperimentalComponentSyntax ?? true,
reactRuntimeTarget,
};
if (babel === true) {
const ast = parse(source, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}"
`);
Expand Down
2 changes: 2 additions & 0 deletions tools/hermes-parser/js/hermes-parser/src/ParserOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,6 +24,7 @@ export const ParserOptionsKeys: $ReadOnlySet<$Keys<ParserOptions>> = new Set([
'babel',
'flow',
'enableExperimentalComponentSyntax',
'reactRuntimeTarget',
'sourceFilename',
'sourceType',
'tokens',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ function createPropsTypeAnnotation(
function mapComponentParameters(
params: $ReadOnlyArray<ComponentParameter | RestElement>,
options: ParserOptions,
): $ReadOnly<{
props: ?(ObjectPattern | Identifier),
ref: ?(BindingName | AssignmentPattern),
Expand All @@ -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<ObjectTypePropertySignature>, ?ObjectTypeSpreadProperty],
Expand Down Expand Up @@ -522,7 +526,10 @@ function createForwardRefWrapper(
};
}

function mapComponentDeclaration(node: ComponentDeclaration): {
function mapComponentDeclaration(
node: ComponentDeclaration,
options: ParserOptions,
): {
comp: FunctionDeclaration,
forwardRefDetails: ?ForwardRefDetails,
} {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -685,9 +692,10 @@ function scanForFirstComponentReference(
function mapComponentDeclarationIntoList(
node: ComponentDeclaration,
newBody: Array<Statement | ModuleDeclaration>,
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(
Expand Down Expand Up @@ -715,12 +723,13 @@ function mapComponentDeclarationIntoList(

function mapStatementList(
stmts: $ReadOnlyArray<Statement | ModuleDeclaration>,
options: ParserOptions,
) {
const newBody: Array<Statement | ModuleDeclaration> = [];
for (const node of stmts) {
switch (node.type) {
case 'ComponentDeclaration': {
mapComponentDeclarationIntoList(node, newBody);
mapComponentDeclarationIntoList(node, newBody, options);
break;
}
case 'HookDeclaration': {
Expand All @@ -733,6 +742,7 @@ function mapStatementList(
mapComponentDeclarationIntoList(
node.declaration,
newBody,
options,
componentOrRef => {
switch (componentOrRef.type) {
case 'FunctionDeclaration': {
Expand Down Expand Up @@ -781,6 +791,7 @@ function mapStatementList(
mapComponentDeclarationIntoList(
node.declaration,
newBody,
options,
componentOrRef => nodeWith(node, {declaration: componentOrRef}),
);
break;
Expand All @@ -806,7 +817,7 @@ function mapStatementList(

export function transformProgram(
program: Program,
_options: ParserOptions,
options: ParserOptions,
): Program {
return SimpleTransform.transformProgram(program, {
transform(node: ESNode) {
Expand All @@ -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': {
Expand Down

0 comments on commit 70df046

Please sign in to comment.