Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_parser): instantiation expressions #3147 #4035

Merged
merged 2 commits into from
Dec 20, 2022

Conversation

denbezrukov
Copy link
Contributor

Summary

This PR fixes:
#3793
#3147

When a potential type argument list is followed by

  • a line break,
  • an ( token,
  • a template literal string, or
  • any token except < or > that isn't the start of an expression,

Typescript reference:

Typescript design notes:

Relative issues:

Open questions:

Test Plan

cargo test -p rome_js_parser

test cases from babel:

test cases from esbuild:

@netlify
Copy link

netlify bot commented Dec 11, 2022

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 2b69507
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/639c15ee0a9ac0000805f32a

@denbezrukov denbezrukov force-pushed the feat/instantiation_expression branch 2 times, most recently from 92efc3c to c88a068 Compare December 11, 2022 12:08
@MichaReiser MichaReiser added L-JavaScript Langauge: JavaScript A-Parser Area: parser labels Dec 12, 2022
@MichaReiser MichaReiser added this to the Next milestone Dec 12, 2022
@MichaReiser
Copy link
Contributor

Let's rebase this PR to get the latest parser conformance tests. This PR should fix some of them.

Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

I ran the typescript instantiation expression tests and there's one test that is passing when it should not:

  • [FAIL] type-arguments\instantiation-expression-property-access: Incorrectly passed

The test should fail because it's invalid to access a property after an instantiation expression.

// invalid
a<b>.c;
a<b>?.c;
a<b>?.[c];

I assume that TypeScript implements this as part of the type checker and not as part of the parser itself (but it would be worth double-checking).

We have two options if that's the case:

  • Try to implement it as part of the parser
  • Implement a syntax rule that reports property accessors after instantiation expressions

And we can implement this either as part of this or a separate PR. Let me know what you prefer.

@denbezrukov
Copy link
Contributor Author

Babel ts parser and typescript parser have a different implementation about access a property after an instantiation expression.

We want to have the same behaviour as typescript parser. Typescript playground

Added new test cases and an error.

@MichaReiser
Copy link
Contributor

MichaReiser commented Dec 20, 2022

Thank you so much for working on this. Well done! 🥳

@MichaReiser MichaReiser merged commit cce872b into rome:main Dec 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Parser Area: parser L-JavaScript Langauge: JavaScript
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants