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

x</x>/ fails to parse #51649

Closed
antimatter15 opened this issue Nov 26, 2022 · 8 comments
Closed

x</x>/ fails to parse #51649

antimatter15 opened this issue Nov 26, 2022 · 8 comments
Labels
Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it

Comments

@antimatter15
Copy link

antimatter15 commented Nov 26, 2022

Bug Report

πŸ”Ž Search Terms

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about it

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

x </x>/

πŸ™ Actual behavior

There is a parse error for a perfectly valid expression

Unexpected keyword or identifier.
Declaration or statement expected.
Unterminated regular expression literal.

πŸ™‚ Expected behavior

It should instead parse the expression as

x < /x>/

This is the behavior that is found in Babel and ESBuild.

@MartinJohns
Copy link
Contributor

Why would you ever write code like this? Any practical example?

@Josh-Cena
Copy link
Contributor

@MartinJohns A parser error is a parser error; it indicates a bug. If TS wants to remain a superset of JS it must parse all valid JS as JS. This seems like a lexing error; I'm not sure if the TS lexer follows the ES model, but the first / should definitely be consumed as InputElementRegExp because a < punctuator, whether as the start of a generic or as the less than operator, cannot be directly followed by a / punctuator.

@MartinJohns
Copy link
Contributor

I'd argue the team has better things to do than to make sure some obscure code that's never really written works. Resources are finite.

@fatcerberus
Copy link

Given that the code parses successfully when adding a space after the <, the bug is likely caused by the ambiguity between JS and JSX interpretations of the same code, which wouldn't be unprecedented (this is not valid JSX because there's no opening <x> tag but error recovery in the parser muddies the waters).

I think there have been similar bugs in the past caused by parsing ambiguities that were deliberately not fixed (or at least not completely) due to design limitations in the parser, so I could definitely see "nobody would write this code" being the justification given for not fixing it. See for example #16241 (an extreme example where the JS and TS interpretations of the code are mutually exclusive) which looks like it did get fixed but took several years and a few attempts to do so.

@jcalz
Copy link
Contributor

jcalz commented Nov 27, 2022

Reminds me of to #33639, which was considered "won't fix" even though it does technically mean that TS isn't exactly a superset of JS.

@Josh-Cena
Copy link
Contributor

Josh-Cena commented Nov 27, 2022

Originally I thought it was confused with instantiation expressions; but ambiguity with JSX seems harder to fix (albeit not impossible). However, #33639 was due to actual grammar ambiguity, but this one is simply a faulty lexer. (Easier said than done though; I don't know if it's trivial to fix, but should be fixable without fumbling with different ambiguous grammar extensions)

@RyanCavanaugh RyanCavanaugh added the Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it label Dec 1, 2022
@RyanCavanaugh
Copy link
Member

If someone can fix this without negative side effects, feel free to send a PR, but this is just a "You tried to break it on purpose and were successful" situation.

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Dec 1, 2022
@fatcerberus
Copy link

this is just a "You tried to break it on purpose and were successful" situation

someone pick up that phone because I called it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it
Projects
None yet
Development

No branches or pull requests

6 participants