Skip to content

Port ?. emit #1385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,20 @@ func (n *Node) Elements() []*Node {
return nil
}

func (n *Node) QuestionDotToken() *Node {
switch n.Kind {
case KindElementAccessExpression:
return n.AsElementAccessExpression().QuestionDotToken
case KindPropertyAccessExpression:
return n.AsPropertyAccessExpression().QuestionDotToken
case KindCallExpression:
return n.AsCallExpression().QuestionDotToken
case KindTaggedTemplateExpression:
return n.AsTaggedTemplateExpression().QuestionDotToken
}
panic("Unhandled case in Node.QuestionDotToken: " + n.Kind.String())
}

// Determines if `n` contains `descendant` by walking up the `Parent` pointers from `descendant`. This method panics if
// `descendant` or one of its ancestors is not parented except when that node is a `SourceFile`.
func (n *Node) Contains(descendant *Node) bool {
Expand Down Expand Up @@ -1673,6 +1687,10 @@ func (n *Node) AsSyntaxList() *SyntaxList {
return n.data.(*SyntaxList)
}

func (n *Node) AsSyntheticReferenceExpression() *SyntheticReferenceExpression {
return n.data.(*SyntheticReferenceExpression)
}

// NodeData

type nodeData interface {
Expand Down Expand Up @@ -4127,6 +4145,53 @@ func IsNotEmittedTypeElement(node *Node) bool {
return node.Kind == KindNotEmittedTypeElement
}

// SyntheticReferenceExpression
// Used by optional chaining transform to shuffle a `this` arg expression between steps of a chain.
// While this does implement the full expected interface of a node, and is used in place of a node in transforms,
// it generally shouldn't be treated or visited like a normal node.

type SyntheticReferenceExpression struct {
ExpressionBase
Expression *Expression
ThisArg *Expression
}

func (f *NodeFactory) NewSyntheticReferenceExpression(expr *Expression, thisArg *Expression) *Node {
data := &SyntheticReferenceExpression{Expression: expr, ThisArg: thisArg}
return newNode(KindSyntheticReferenceExpression, data, f.hooks)
}

func (f *NodeFactory) UpdateSyntheticReferenceExpression(node *SyntheticReferenceExpression, expr *Expression, thisArg *Expression) *Node {
if expr != node.Expression || thisArg != node.ThisArg {
return updateNode(f.NewSyntheticReferenceExpression(expr, thisArg), node.AsNode(), f.hooks)
}
return node.AsNode()
}

func (node *SyntheticReferenceExpression) ForEachChild(v Visitor) bool {
return visit(v, node.Expression)
}

func (node *SyntheticReferenceExpression) VisitEachChild(v *NodeVisitor) *Node {
return v.Factory.UpdateSyntheticReferenceExpression(node, v.visitNode(node.Expression), node.ThisArg)
}

func (node *SyntheticReferenceExpression) Clone(f NodeFactoryCoercible) *Node {
return cloneNode(f.AsNodeFactory().NewSyntheticReferenceExpression(node.Expression, node.ThisArg), node.AsNode(), f.AsNodeFactory().hooks)
}

func (node *SyntheticReferenceExpression) computeSubtreeFacts() SubtreeFacts {
return propagateSubtreeFacts(node.Expression)
}

func (node *SyntheticReferenceExpression) propagateSubtreeFacts() SubtreeFacts {
return node.SubtreeFacts()
}

func IsSyntheticReferenceExpression(node *Node) bool {
return node.Kind == KindSyntheticReferenceExpression
}

// ImportEqualsDeclaration

type ImportEqualsDeclaration struct {
Expand Down
8 changes: 8 additions & 0 deletions internal/printer/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,14 @@ func (f *NodeFactory) NewGlobalMethodCall(globalObjectName string, methodName st
return f.NewMethodCall(f.NewIdentifier(globalObjectName), f.NewIdentifier(methodName), argumentsList)
}

func (f *NodeFactory) NewFunctionCallCall(target *ast.Expression, thisArg *ast.Expression, argumentsList []*ast.Node) *ast.Node {
if thisArg == nil {
panic("Attempted to construct function call call without this argument expression")
}
args := append([]*ast.Expression{thisArg}, argumentsList...)
return f.NewMethodCall(target, f.NewIdentifier("call"), args)
}

// Determines whether a node is a parenthesized expression that can be ignored when recreating outer expressions.
//
// A parenthesized expression can be ignored when all of the following are true:
Expand Down
4 changes: 2 additions & 2 deletions internal/printer/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3093,12 +3093,12 @@ func (p *Printer) emitExpression(node *ast.Expression, precedence ast.OperatorPr
return
case ast.KindPartiallyEmittedExpression:
p.emitPartiallyEmittedExpression(node.AsPartiallyEmittedExpression(), precedence)
case ast.KindSyntheticReferenceExpression:
panic("SyntheticReferenceExpression should not be printed")

// !!!
////case ast.KindCommaListExpression:
//// p.emitCommaList(node.AsCommaListExpression())
////case ast.KindSyntheticReferenceExpression:
//// return Debug.fail("SyntheticReferenceExpression should not be printed")

default:
panic(fmt.Sprintf("unexpected Expression: %v", node.Kind))
Expand Down
217 changes: 216 additions & 1 deletion internal/transformers/estransforms/optionalchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,222 @@ type optionalChainTransformer struct {
}

func (ch *optionalChainTransformer) visit(node *ast.Node) *ast.Node {
return node // !!!
if node.SubtreeFacts()&ast.SubtreeContainsOptionalChaining == 0 {
return node
}
switch node.Kind {
case ast.KindCallExpression:
return ch.visitCallExpression(node.AsCallExpression(), false)
case ast.KindPropertyAccessExpression,
ast.KindElementAccessExpression:
if node.Flags&ast.NodeFlagsOptionalChain != 0 {
return ch.visitOptionalExpression(node, false, false)
}
return ch.Visitor().VisitEachChild(node)
case ast.KindDeleteExpression:
return ch.visitDeleteExpression(node.AsDeleteExpression())
default:
return ch.Visitor().VisitEachChild(node)
}
}

func (ch *optionalChainTransformer) visitCallExpression(node *ast.CallExpression, captureThisArg bool) *ast.Node {
if node.Flags&ast.NodeFlagsOptionalChain != 0 {
// If `node` is an optional chain, then it is the outermost chain of an optional expression.
return ch.visitOptionalExpression(node.AsNode(), captureThisArg, false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you restore the isDelete comments?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can... but the go formatting for it is weird af, placing the comment before the preceding comma. I've been avoiding most of those comments since I noticed it. With only two callsites that aren't named variables (both of which pass false), I'm not too worried about this one being too confusing.

}
if ast.IsParenthesizedExpression(node.Expression) {
unwrapped := ast.SkipParentheses(node.Expression)
if unwrapped.Flags&ast.NodeFlagsOptionalChain != 0 {
// capture thisArg for calls of parenthesized optional chains like `(foo?.bar)()`
expression := ch.visitParenthesizedExpression(node.Expression.AsParenthesizedExpression(), true, false)
args := ch.Visitor().VisitNodes(node.Arguments)
if ast.IsSyntheticReferenceExpression(expression) {
res := ch.Factory().NewFunctionCallCall(expression.AsSyntheticReferenceExpression().Expression, expression.AsSyntheticReferenceExpression().ThisArg, args.Nodes)
res.Loc = node.Loc
ch.EmitContext().SetOriginal(res, node.AsNode())
return res
}
return ch.Factory().UpdateCallExpression(node, expression, nil, nil, args)
}
}
return ch.Visitor().VisitEachChild(node.AsNode())
}

func (ch *optionalChainTransformer) visitParenthesizedExpression(node *ast.ParenthesizedExpression, captureThisArg bool, isDelete bool) *ast.Node {
expr := ch.visitNonOptionalExpression(node.Expression, captureThisArg, isDelete)
if ast.IsSyntheticReferenceExpression(expr) {
// `(a.b)` -> { expression `((_a = a).b)`, thisArg: `_a` }
// `(a[b])` -> { expression `((_a = a)[b])`, thisArg: `_a` }
synth := expr.AsSyntheticReferenceExpression()
res := ch.Factory().NewSyntheticReferenceExpression(ch.Factory().UpdateParenthesizedExpression(node, synth.Expression), synth.ThisArg)
ch.EmitContext().SetOriginal(res, node.AsNode())
return res
}
return ch.Factory().UpdateParenthesizedExpression(node, expr)
}

func (ch *optionalChainTransformer) visitPropertyOrElementAccessExpression(node *ast.Expression, captureThisArg bool, isDelete bool) *ast.Expression {
if node.Flags&ast.NodeFlagsOptionalChain != 0 {
// If `node` is an optional chain, then it is the outermost chain of an optional expression.
return ch.visitOptionalExpression(node.AsNode(), captureThisArg, isDelete)
}
expression := ch.Visitor().VisitNode(node.Expression())
// Debug.assertNotNode(expression, isSyntheticReference); // !!!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anything go wrong if you panic here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These asserts were to satisfy the TS typechecker and remove a type from a union type. Technically, they're not actually required anymore, but if they're added back, they should be in the same systematic debug-build-specific style, not as ad-hoc panics.


var thisArg *ast.Expression
if captureThisArg {
if !transformers.IsSimpleCopiableExpression(expression) {
thisArg = ch.Factory().NewTempVariable()
ch.EmitContext().AddVariableDeclaration(thisArg)
expression = ch.Factory().NewAssignmentExpression(thisArg, expression)
} else {
thisArg = expression
}
}

if node.Kind == ast.KindPropertyAccessExpression {
p := node.AsPropertyAccessExpression()
expression = ch.Factory().UpdatePropertyAccessExpression(p, expression, nil, ch.Visitor().VisitNode(p.Name()))
} else {
p := node.AsElementAccessExpression()
expression = ch.Factory().UpdateElementAccessExpression(p, expression, nil, ch.Visitor().VisitNode(p.AsElementAccessExpression().ArgumentExpression))
}

if thisArg != nil {
res := ch.Factory().NewSyntheticReferenceExpression(expression, thisArg)
ch.EmitContext().SetOriginal(res, node.AsNode())
return res
}
return expression
}

func (ch *optionalChainTransformer) visitDeleteExpression(node *ast.DeleteExpression) *ast.Node {
unwrapped := ast.SkipParentheses(node.Expression)
if unwrapped.Flags&ast.NodeFlagsOptionalChain != 0 {
return ch.visitNonOptionalExpression(node.Expression, false, true)
}
return ch.Visitor().VisitEachChild(node.AsNode())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you do this instead of the original-ish

Suggested change
return ch.Visitor().VisitEachChild(node.AsNode())
deletedExpr := ch.Visitor().VisitNode(node.Expression)
return ch.Factory().UpdateDeleteExpression(node, deletedExpr)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because they're equivalent (as best I can tell). Why manually pull out and update the child of the node, when visitEachChild does that already.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, though it's technically doing a bit more work under the hood.

}

func (ch *optionalChainTransformer) visitNonOptionalExpression(node *ast.Expression, captureThisArg bool, isDelete bool) *ast.Expression {
switch node.Kind {
case ast.KindParenthesizedExpression:
return ch.visitParenthesizedExpression(node.AsParenthesizedExpression(), captureThisArg, isDelete)
case ast.KindElementAccessExpression, ast.KindPropertyAccessExpression:
return ch.visitPropertyOrElementAccessExpression(node, captureThisArg, isDelete)
case ast.KindCallExpression:
return ch.visitCallExpression(node.AsCallExpression(), captureThisArg)
default:
return ch.Visitor().VisitNode(node.AsNode())
}
}

type flattenResult struct {
expression *ast.Expression
chain []*ast.Node
}

func flattenChain(chain *ast.Node) flattenResult {
// Debug.assertNotNode(chain, isNonNullChain); // !!!
links := []*ast.Node{chain}
for !ast.IsTaggedTemplateExpression(chain) && chain.QuestionDotToken() == nil {
chain = ast.SkipPartiallyEmittedExpressions(chain.Expression())
// Debug.assertNotNode(chain, isNonNullChain); // !!!
links = append([]*ast.Node{chain}, links...)
}
Comment on lines +136 to +137
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this was the original but it seems so not-ideal - why didn't we just keep appending and reverse?

Suggested change
links = append([]*ast.Node{chain}, links...)
}
links = append(links, chain)
}
slices.Reverse(links)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More often than not, these flattened chains are single element - it's not often people do a a?.b?.c?.d deal. Given that, I think the original is fine for the single or even two element cases. It's not something I'd think needs optimization, especially since reversing a slice isn't great. If anything, you'd do a quick decent to count the chain length, preallocate the slice (probably from a pool, if this was actually hot code), and then insert directly at target position as you descend.

return flattenResult{chain.Expression(), links}
}

func isCallChain(node *ast.Node) bool {
return ast.IsCallExpression(node) && node.Flags&ast.NodeFlagsOptionalChain != 0
}

func (ch *optionalChainTransformer) visitOptionalExpression(node *ast.Node, captureThisArg bool, isDelete bool) *ast.Node {
r := flattenChain(node)
expression := r.expression
chain := r.chain
left := ch.visitNonOptionalExpression(ast.SkipPartiallyEmittedExpressions(expression), isCallChain(chain[0]), false)
var leftThisArg *ast.Expression
capturedLeft := left
if ast.IsSyntheticReferenceExpression(left) {
leftThisArg = left.AsSyntheticReferenceExpression().ThisArg
capturedLeft = left.AsSyntheticReferenceExpression().Expression
Comment on lines +152 to +154
Copy link
Member

@DanielRosenwasser DanielRosenwasser Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ast.IsSyntheticReferenceExpression(left) {
leftThisArg = left.AsSyntheticReferenceExpression().ThisArg
capturedLeft = left.AsSyntheticReferenceExpression().Expression
if ast.IsSyntheticReferenceExpression(left) {
left := left.AsSyntheticReferenceExpression()
leftThisArg = left.ThisArg
capturedLeft = left.Expression

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that kind of shadowing is a great idea (left the node being shadowed by left the node data) - don't we have a lint rule forbidding it? Two uses like this kinda aren't enough to bother, anyway, since it's not clarifying or simplifying much of anything. Unless someone shows me the casts having an outsized impact on perf, which is going to make me scream, since old casts were free-and-erased so never really needed perf scrutiny.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the calls and casts have cost, I'm afraid

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(but really, don't think about it)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just gets a bit unwieldy, and I end up thinking about it :D

}
leftExpression := ch.Factory().RestoreOuterExpressions(expression, capturedLeft, ast.OEKPartiallyEmittedExpressions)
if !transformers.IsSimpleCopiableExpression(capturedLeft) {
capturedLeft = ch.Factory().NewTempVariable()
ch.EmitContext().AddVariableDeclaration(capturedLeft)
leftExpression = ch.Factory().NewAssignmentExpression(capturedLeft, leftExpression)
}
rightExpression := capturedLeft
var thisArg *ast.Expression

for i, segment := range chain {
switch segment.Kind {
case ast.KindElementAccessExpression, ast.KindPropertyAccessExpression:
if i == len(chain)-1 && captureThisArg {
if !transformers.IsSimpleCopiableExpression(rightExpression) {
thisArg = ch.Factory().NewTempVariable()
ch.EmitContext().AddVariableDeclaration(thisArg)
rightExpression = ch.Factory().NewAssignmentExpression(thisArg, rightExpression)
} else {
thisArg = rightExpression
}
}
if segment.Kind == ast.KindElementAccessExpression {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - the check is inverted compared to the original code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As are the bodies in the branches of the check. E comes before P, so E comes first in the case clause list, so handling the first case explicitly is a bit more natural looking.

rightExpression = ch.Factory().NewElementAccessExpression(rightExpression, nil, ch.Visitor().VisitNode(segment.AsElementAccessExpression().ArgumentExpression), ast.NodeFlagsNone)
} else {
rightExpression = ch.Factory().NewPropertyAccessExpression(rightExpression, nil, ch.Visitor().VisitNode(segment.AsPropertyAccessExpression().Name()), ast.NodeFlagsNone)
}
case ast.KindCallExpression:
if i == 0 && leftThisArg != nil {
if !ch.EmitContext().HasAutoGenerateInfo(leftThisArg) {
leftThisArg = leftThisArg.Clone(ch.Factory())
ch.EmitContext().AddEmitFlags(leftThisArg, printer.EFNoComments)
}
callThisArg := leftThisArg
if leftThisArg.Kind == ast.KindSuperKeyword {
callThisArg = ch.Factory().NewThisExpression()
}
rightExpression = ch.Factory().NewFunctionCallCall(rightExpression, callThisArg, ch.Visitor().VisitNodes(segment.ArgumentList()).Nodes)
} else {
rightExpression = ch.Factory().NewCallExpression(
rightExpression,
nil,
nil,
ch.Visitor().VisitNodes(segment.ArgumentList()),
ast.NodeFlagsNone,
)
}
}
ch.EmitContext().SetOriginal(rightExpression, segment)
}

var target *ast.Node
if isDelete {
target = ch.Factory().NewConditionalExpression(
createNotNullCondition(ch.EmitContext(), leftExpression, capturedLeft, true),
ch.Factory().NewToken(ast.KindQuestionToken),
ch.Factory().NewTrueExpression(),
ch.Factory().NewToken(ast.KindColonToken),
ch.Factory().NewDeleteExpression(rightExpression),
)
} else {
target = ch.Factory().NewConditionalExpression(
createNotNullCondition(ch.EmitContext(), leftExpression, capturedLeft, true),
ch.Factory().NewToken(ast.KindQuestionToken),
ch.Factory().NewVoidZeroExpression(),
ch.Factory().NewToken(ast.KindColonToken),
rightExpression,
)
}
target.Loc = node.Loc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you have to do this? I see you run SetOriginal farther below. The ordering is a little bit different here though. I would have thought you'd run SetOriginal here based on the original code.

Copy link
Member Author

@weswigham weswigham Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.Loc = .Loc is equivalent to SetTextRange, which is independent from SetOriginalNode (text ranges control comment and source map spans, original nodes control emit resolver API nodes and in some cases fallback location spans) - and the original nodes are set in new places now because we now have an assert if we try and set original multiple times (to different things), so all these codepaths now reliably set originals when they're done constructing the new node, rather than sometimes doing it and sometimes letting the caller handle it.

if thisArg != nil {
target = ch.Factory().NewSyntheticReferenceExpression(target, thisArg)
}
ch.EmitContext().SetOriginal(target, node.AsNode())
return target
}

func newOptionalChainTransformer(emitContext *printer.EmitContext) *transformers.Transformer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ const animal = { type: 'cat', canMeow: true };
assertEqual(animal.type, 'cat');
animal.canMeow; // since is cat, should not be an error
const animalOrUndef = { type: 'cat', canMeow: true };
assertEqual(animalOrUndef?.type, 'cat');
assertEqual(animalOrUndef === null || animalOrUndef === void 0 ? void 0 : animalOrUndef.type, 'cat');
animalOrUndef.canMeow; // since is cat, should not be an error
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,4 @@
-"use strict";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this very common diff is an easy fix somewhere...

Copy link
Member Author

@weswigham weswigham Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a script file by strada logic. (no exports/imports, nothing forcing module mode parsing.) In corsa, the module transforms add use strict if needed, but in strada, the ts transform does (via visitLexicalEnvironment in visitSourceFile) so it's done for script files (and I think the module transforms, redundantly, also do). Obviously the module transforms don't touch script files, so they don't add the prologue here.

const animal = { type: 'cat', canMeow: true };
assertEqual(animal.type, 'cat');
animal.canMeow; // since is cat, should not be an error
const animalOrUndef = { type: 'cat', canMeow: true };
-assertEqual(animalOrUndef === null || animalOrUndef === void 0 ? void 0 : animalOrUndef.type, 'cat');
+assertEqual(animalOrUndef?.type, 'cat');
animalOrUndef.canMeow; // since is cat, should not be an error
animal.canMeow; // since is cat, should not be an error
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ new A()?.b() // ok


//// [invalidOptionalChainFromNewExpression.js]
var _a, _b;
class A {
b() { }
}
(new A)?.b(); // error
new A()?.b(); // ok
(_a = new A) === null || _a === void 0 ? void 0 : _a.b(); // error
(_b = new A()) === null || _b === void 0 ? void 0 : _b.b(); // ok

This file was deleted.

Loading
Loading