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 value being parsed as a TS type argument #36662

Closed
shicks opened this issue Feb 6, 2020 · 4 comments · Fixed by #36673
Closed

JS value being parsed as a TS type argument #36662

shicks opened this issue Feb 6, 2020 · 4 comments · Fixed by #36673
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript
Milestone

Comments

@shicks
Copy link
Contributor

shicks commented Feb 6, 2020

TypeScript Version: 3.8.0-dev.20200205

Search Terms: emscripten "type arguments"

Code

The following code (I added the declaration) was found in the distribution package for viz.js: https://github.com/mdaines/viz.js/releases/download/v2.1.2/lite.render.js. This file is generated by emscripten, so the syntax is a little weird.

declare let [k, e, a, c, n]: number[];
if ((k | 0) < (e | 0) | (k | 0) > (a << 16 >> 16 | 0)) {
              c[n >> 2] = 1364;
}

Expected behavior:

This is valid JavaScript and should have the same parse as JS.

Actual behavior:

If this is parsed as TS, the part between the < and the > get pulled out as a type annotation, leaving

if ((k | 0)(a << 16 >> 16 | 0))

which is clearly incorrect, as it's trying to call a number. If this is parsed as a JS file (with --allowJs), it produces an error:

$ tsc --allowJs a.js
a.js:2:14 - error TS8011: 'type arguments' can only be used in a .ts file.

2 if ((k | 0) < (e | 0) | (k | 0) > (a << 16 >> 16 | 0)) {
               ~~~~~~~~~~~~~~~~~~

Even if one makes the argument that this is not valid TS syntax, it should still parse the explicitly-JS input.

Playground Link:

https://www.typescriptlang.org/play/?ts=3.8.0-dev.20200205&ssl=1&ssc=1&pln=4&pc=2#code/CYUwxgNghgTiAEEQBd4G0DWAaeIdRzBwDsBdALnmIFcBbAIxBjVIG4AoASwDN4AKPhngAfeAAYAlPAA8-BKMkj+QhVIB8-KDNkBGAGzw1G-UslSA3u3jWbt22DTFDGgEyl4AXng6AzHoAsHAC+QA

Related Issues:

I was unable to find anything.

@jcalz
Copy link
Contributor

jcalz commented Feb 7, 2020

Duplicate of #33639, except that this was spotted in the wild.

@DanielRosenwasser
Copy link
Member

Thanks for the de-dupe - though I think there's a good point which is that this isn't a .ts file, it's a .js file. Maybe we should have a separate path for parsing in .js files.

@shicks can you tell us more about why you ended up piping this through TypeScript?

@shicks
Copy link
Contributor Author

shicks commented Feb 7, 2020

We have a ts_development_srcs bazel build rule that uses tsc to transpile/downlevel the transitive closure of a project's dependencies for testing and development serving (as opposed to the production compilation path that uses tsickle without any downleveling so that Closure Compiler can do better optimizations). This build rule provides an option (on by default) to pass *.js sources to tsc to downlevel as well, so that the whole transitive closure can be handled in one place (we have build rules for using Closure to downlevel JS only, but they don't work well with TS inputs and we're trying to move off of them anyway).

The workaround here was to just turn off this option, but that could be a problem if there were also *.js sources written in ESnext sitting alongside viz.js and needing to be tested in an older browser.

@fatcerberus
Copy link

Even if one makes the argument that this is not valid TS syntax

If only it were! The fun thing here is that this is valid TS syntax. < (e | 0) | (k | 0) > is most likely being parsed as e | 0 | k | 0, that is, a union type. In a .ts file, the expression is actually ambiguous. Fixing it is likely nontrivial ☹️ (though take my outsider perspective with a grain of salt, I don't know that much about the compiler internals!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants