Skip to content
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

Remove JSXMemberExpressionObject::Identifier variant #5353

Closed
overlookmotel opened this issue Aug 30, 2024 · 3 comments · Fixed by #5356 or #5358
Closed

Remove JSXMemberExpressionObject::Identifier variant #5353

overlookmotel opened this issue Aug 30, 2024 · 3 comments · Fixed by #5356 or #5358
Labels
A-ast Area - AST

Comments

@overlookmotel
Copy link
Contributor

overlookmotel commented Aug 30, 2024

Currently we parse all JSX member expressions as an IdentifierReference followed by one or more JSXIdentifiers.

This is the case regardless of whether the first identifier is upper case or not. i.e. foo in <foo.bar> is interpreted as an IdentifierReference, even though in <foo> it'd be a plain IdentifierName.

This behavior aligns with Babel: Babel REPL

Therefore we can remove the JSXMemberExpressionObject::Identifier variant - it's not possible to create one - it's always an IdentifierReference, another nested JSXMemberExpression, or this (#5352).

It'll be easier to do this once we've closed #5352.

@Dunqing
Copy link
Member

Dunqing commented Aug 30, 2024

I've considered it, but it doesn't work. In this case: <A.B.C />, the A is an IdentifierReference, but B isn't. Therefore, we need to keep it for nested JSXMemberExpression.

@overlookmotel
Copy link
Contributor Author

I think it still works for <A.B.C />. The nesting is in the object field of JSXMemberExpression.

So in <A.B.C />, B and C are the property field and are JSXIdentifiers. It looks like this:

JSXMemberExpression {
    object: JSXMemberExpressionObject::MemberExpression(JSXMemberExpression {
        object: JSXMemberExpressionObject::IdentifierReference( IdentifierReference(A) ),
        property: JSXIdentifier(B),
    }),
    property: JSXIdentifier(C),
}

So, unless I'm mistaken, there are no circumstances in which JSXMemberExpressionObject::Identifier variant is ever generated by parser.

@Dunqing
Copy link
Member

Dunqing commented Aug 30, 2024

Ahh, My brain is broken. You are right.

I've made a mistake here

JSXMemberExpressionObject::IdentifierReference(_) => {
unreachable!()
}

Doesn't have any tests failed 🥲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST
Projects
None yet
3 participants