diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index 4873441441..253817287d 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -1796,39 +1796,67 @@ function lowerMemberExpression( ): { object: Place; property: Place | string; value: InstructionValue } { const exprNode = expr.node; const exprLoc = exprNode.loc ?? GeneratedSource; - const object = lowerExpressionToTemporary(builder, expr.get("object")); - const property = expr.get("property"); + const objectNode = expr.get("object"); + const propertyNode = expr.get("property"); + const object = lowerExpressionToTemporary(builder, objectNode); + + if ( + objectNode.isOptionalMemberExpression() && + !expr.isOptionalMemberExpression() + ) { + // Babel's `isOptionalMemberExpression` indicates whether this property load itself + // is conditional (i.e. within an "optional chain"). This is different from the + // `optional` property, which is only true for property loads at the start of an + // optional chain. e.g. `a.b?.c.d` decomposes into + // [0] MemberExpr a.b; // not in an optional chain + // [1] OptionalMemberExpr [0]?.c + // [2] OptionalMemberExpr [1].c + // [3] OptionalMemberExpr [2].d + // We currently do not handle non-conditional loads from an optional memberexpr + // e.g. `(a?.b).c` + // See error.nonoptional-load-from-optional-memberexpr test fixture for details + builder.errors.push({ + reason: `(BuildHIR::lowerMemberExpression) Handle optional chaining for non-optional member expr.`, + severity: ErrorSeverity.Todo, + nodePath: propertyNode, + }); + return { + object, + property: propertyNode.toString(), + value: { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }, + }; + } if (!expr.node.computed) { - if (!property.isIdentifier()) { + if (!propertyNode.isIdentifier()) { builder.errors.push({ - reason: `(BuildHIR::lowerMemberExpression) Handle ${property.type} property`, + reason: `(BuildHIR::lowerMemberExpression) Handle ${propertyNode.type} property`, severity: ErrorSeverity.Todo, - nodePath: property, + nodePath: propertyNode, }); return { object, - property: property.toString(), + property: propertyNode.toString(), value: { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }, }; } const value: InstructionValue = { kind: "PropertyLoad", object: { ...object }, - property: property.node.name, + property: propertyNode.node.name, loc: exprLoc, optional: expr.node.optional ?? false, }; - return { object, property: property.node.name, value }; + return { object, property: propertyNode.node.name, value }; } else { - if (!property.isExpression()) { + if (!propertyNode.isExpression()) { builder.errors.push({ - reason: `(BuildHIR::lowerMemberExpression) Expected Expression, got ${property.type} property`, + reason: `(BuildHIR::lowerMemberExpression) Expected Expression, got ${propertyNode.type} property`, severity: ErrorSeverity.InvalidInput, - nodePath: property, + nodePath: propertyNode, }); return { object, - property: property.toString(), + property: propertyNode.toString(), value: { kind: "UnsupportedNode", node: exprNode, @@ -1843,7 +1871,7 @@ function lowerMemberExpression( nodePath: expr, }); } - const propertyPlace = lowerExpressionToTemporary(builder, property); + const propertyPlace = lowerExpressionToTemporary(builder, propertyNode); const value: InstructionValue = { kind: "ComputedLoad", object: { ...object }, diff --git a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 05697c3a1b..0a8681a95f 100644 --- a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -752,17 +752,22 @@ function codegenInstructionValue( break; } case "PropertyLoad": { - if (instrValue.optional) { + const object = codegenPlace(cx, instrValue.object); + // We currently only lower single chains of optional memberexpr. + // (See BuildHIR.ts for more detail.) + if (t.isOptionalMemberExpression(object) || instrValue.optional) { value = t.optionalMemberExpression( - codegenPlace(cx, instrValue.object), + object, t.identifier(instrValue.property), undefined, - true + instrValue.optional ); } else { value = t.memberExpression( - codegenPlace(cx, instrValue.object), - t.identifier(instrValue.property) + object, + t.identifier(instrValue.property), + undefined, + instrValue.optional ); } break; diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.expect.md new file mode 100644 index 0000000000..42ff978268 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.expect.md @@ -0,0 +1,30 @@ + +## Input + +```javascript +// Note that `a?.b.c` is semantically different from `(a?.b).c` +// Here, 'props?.a` is an optional chain, and `.b` is an unconditional load +// (nullthrows if a is nullish) + +function Component(props) { + let x = (props?.a).b; + return x; +} + +``` + + +## Error + +``` +[ReactForget] TodoError: (BuildHIR::lowerMemberExpression) Handle optional chaining for non-optional member expr. + 4 | + 5 | function Component(props) { +> 6 | let x = (props?.a).b; + | ^ + 7 | return x; + 8 | } + 9 | +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.js b/compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.js new file mode 100644 index 0000000000..87579ff2b2 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.js @@ -0,0 +1,8 @@ +// Note that `a?.b.c` is semantically different from `(a?.b).c` +// Here, 'props?.a` is an optional chain, and `.b` is an unconditional load +// (nullthrows if a is nullish) + +function Component(props) { + let x = (props?.a).b; + return x; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/nested-optional-member-expr.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/nested-optional-member-expr.expect.md index d2bb09a4f8..f7405a2052 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/nested-optional-member-expr.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/nested-optional-member-expr.expect.md @@ -21,7 +21,7 @@ function Component(props) { const c_0 = $[0] !== props.a; let t0; if (c_0) { - t0 = foo((props.a?.b).c.d); + t0 = foo(props.a?.b.c.d); $[0] = props.a; $[1] = t0; } else { diff --git a/compiler/forget/src/__tests__/fixtures/compiler/_bug.optional-chaining-memberexpr.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.expect.md similarity index 86% rename from compiler/forget/src/__tests__/fixtures/compiler/_bug.optional-chaining-memberexpr.expect.md rename to compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.expect.md index b07e0c1380..5d0661286c 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/_bug.optional-chaining-memberexpr.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.expect.md @@ -6,7 +6,7 @@ // We should codegen the correct member expressions function Component(props) { let x = props?.b.c; - let y = (props?.x).y; + let y = props?.b.c.d?.e.f.g?.h; return { x, y }; } @@ -19,8 +19,8 @@ function Component(props) { // We should codegen the correct member expressions function Component(props) { const $ = React.unstable_useMemoCache(3); - const x = (props?.b).c; - const y = (props?.x).y; + const x = props?.b.c; + const y = props?.b.c.d?.e.f.g?.h; const c_0 = $[0] !== x; const c_1 = $[1] !== y; let t0; diff --git a/compiler/forget/src/__tests__/fixtures/compiler/_bug.optional-chaining-memberexpr.js b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.js similarity index 84% rename from compiler/forget/src/__tests__/fixtures/compiler/_bug.optional-chaining-memberexpr.js rename to compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.js index b3e9a1c49b..568d059be8 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/_bug.optional-chaining-memberexpr.js +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.js @@ -2,6 +2,6 @@ // We should codegen the correct member expressions function Component(props) { let x = props?.b.c; - let y = (props?.x).y; + let y = props?.b.c.d?.e.f.g?.h; return { x, y }; }