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

JS files should parse as JS, not as JSX #45166

Open
dcsaszar opened this issue Jul 23, 2021 · 8 comments
Open

JS files should parse as JS, not as JSX #45166

dcsaszar opened this issue Jul 23, 2021 · 8 comments
Assignees
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@dcsaszar
Copy link

dcsaszar commented Jul 23, 2021

Bug Report

🔎 Search Terms

TS1005 allowJS

🕗 Version & Regression Information

3.3.3

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about allowJS and TS1005
  • I was unable to test this on prior versions because playground

⏯ Playground Link

https://www.typescriptlang.org/play?jsx=0&ts=3.3.3&filetype=js#code/C4JwrgpgPABA9AHQEZwHTAgZ2ACgER4CU8cMA8gNIBQokUiK6WuBxcpAYgIICSAMlSA

💻 Code

tsc --allowJs --checkJs test.js

true< /\b/.test("") // OK
true</\b/.test("") // FAIL

This is a small reproducing example.
The real-world code causing this issue is: https://unpkg.com/froala-editor@4.0.3/js/froala_editor.min.js

🙁 Actual behavior

:2:2 - error TS1005: ';' expected.
:2:7 - error TS1127: Invalid character.
:2:10 - error TS1109: Expression expected.

🙂 Expected behavior

Compiles successfully, because line 1 and line 2 are semantically equal in JS.

@dcsaszar
Copy link
Author

Closing, because I realized that it's caused by the JSX setting, which defaults to React in the playground

@dcsaszar
Copy link
Author

dcsaszar commented Jul 24, 2021

Reopening for discussion, because I wasn't able to successfully compile this on the command line.

Although it works with a TS source
tsc --allowJs --noEmit test.ts,

I'm looking for a way to compile JS
tsc --allowJs --noEmit test.js

In #44442 I found statements that explain the current behavior:

  • In .js files we always parse JSX anyway.
    • Were trying to be "prescriptive", not "descriptive" - not something we're in love with.
  • What is the distinction between .js and .jsx in today's implementation?
    • None.

@dcsaszar dcsaszar reopened this Jul 24, 2021
@DanielRosenwasser
Copy link
Member

Note I had the prescriptive/descriptive thing backwards so I've updated that text.

@andrewbranch andrewbranch added the Needs Investigation This issue needs a team member to investigate its status. label Jul 26, 2021
@andrewbranch andrewbranch added this to the TypeScript 4.5.0 milestone Jul 26, 2021
@sandersn sandersn changed the title Parse error with less-than followed by slash (allowJS) In JS, parse error with less-than followed by slash Jul 30, 2021
@sandersn
Copy link
Member

I can't think of any other good way for the scanner to decide whether the characters </ should be scanned as LessThanSlashToken or not. Making the scanner context-sensitive for this case is difficult. There would need to be a good reason for the scanner to support JSX and no-space regex comparisons with the same parser.

I searched through all the JS files that get installed with a full install of Definitely Typed -- the dependencies of over 7000 projects. I couldn't find any example of people writing </\w+?/ on purpose. Just one JSDoc typo, a couple of examples in a comment in a parser, and some strings containing codegen for a different language.

Looking for closing tags in JS source turns up a small number of projects:

  • @emotion/styled, @emotion/core
  • @wordpress/element (test)
  • aphrodite (examples)
  • babel-plugin-add-react-displayname (test)
  • markdown-to-jsx
  • react-native
  • react-native-fs (test?)
  • react-native-gesture-handler
  • react-native-svg
  • semantic-ui-react

But they're widely used, and the number that have JSX in their JS tests implies to me that they expect their users to commonly put JSX in JS.

I think at present the best thing to do is to keep parsing JS as JSX, and continue to be prescriptive rather than descriptive.

@sandersn sandersn removed this from the TypeScript 4.5.0 milestone Oct 28, 2021
@sandersn sandersn added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Oct 28, 2021
@sandersn sandersn changed the title In JS, parse error with less-than followed by slash JS files should parse as JS, not as JSX Oct 28, 2021
@dcsaszar
Copy link
Author

people writing </\w+?/ on purpose

The real-world case mentioned in my initial comment is not caused by people writing this on purpose, but by a minifier/uglifier compressing JS to this form. </\b is still present in https://unpkg.com/froala-editor@4.0.5/js/froala_editor.min.js, which triggers the issue in our case.

@DanielRosenwasser
Copy link
Member

Also, keep in mind that the issue is not that the code compares something with a regex (implausible), but rather that the original code is effectively -1 < /someRegex/.text(someStuff) (plausible).

@sandersn
Copy link
Member

sandersn commented Oct 29, 2021

-1 < /someRegex/.test(someStuff) is a type error at compile-time, and is always true at runtime. There's only one number-typed property on Regex: lastIndex.

@dcsaszar
Copy link
Author

I've had a look at the code and the trick used by the optimizer is that here -1 < makes the following && expression execute unconditionally: -1</x/g.test("y")&&console.log("OK")
In other words, it disposes the return value of /someRegex/.test(someStuff) to make the code shorter.

There's only one number-typed property

In theory, there are other (maybe unlikely in the real world) ways for "number < regex", for example, 4</4/g.test("5").toString().length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants