ember-cli-typescript
ember-cli-typescript copied to clipboard
TypeScript hangs with Promise.all(array) (With ArrayPrototypeExtensions)
Which package(s) does this problem pertain to?
- [ ] @types/ember
- [ ] @types/ember__string
- [ ] @types/ember__polyfills
- [x] @types/ember__object
- [ ] @types/ember__utils
- [x] @types/ember__array
- [ ] @types/ember__engine
- [ ] @types/ember__debug
- [ ] @types/ember__runloop
- [ ] @types/ember__error
- [ ] @types/ember__controller
- [ ] @types/ember__component
- [ ] @types/ember__routing
- [ ] @types/ember__application
- [ ] @types/ember__test
- [ ] @types/ember__service
- [ ] @types/ember-data
- [ ] @types/rsvp
- [ ] @types/ember-test-helpers
- [ ] @types/ember-testing-helpers
- [ ] Other
- [ ] I don't know
What are instructions we can follow to reproduce the issue?
A small repo demonstrating a slow-down combining Promise.all
while having interface Array<T> extends Ember.ArrayPrototypeExtensions<T> {}
. Created it with as few moving parts as possible, so there is basically no code nor an ember app.
git clone [email protected]:mogstad/typescript-ember-slow-example.git
cd typescript-ember-slow-example
npm install
npm start
Now about that bug. What did you expect to see?
I expected the TypeScript code to be compiled swiftly.
What happened instead?
Running it on TS 3.0.3 takes 45 seconds, and 3.1.1 takes 8 minutes. We're only seeing this issue using @types/ember
>= 3.
Workarounds
We've found a few workarounds, by either defining the generic type of Promise.all<T>
(and its friends). (Same issue if using rsvp
)
let items = await Promise.all<string>(promises);
Or only extending MutableArray<T>
instead of ArrayPrototypeExtensions
, we're not using anything in the Observable
interface.
@dwickern or @mike-north do you have any idea what's going on here? It's obviously an upstream issue in TS, but I'm also curious if we can at least identify why it's an issue so we can give them a good and useful report.
Ran into this issue as well, it's pretty frustrating haha, since there is no messaging and the build just hangs.
Same here. Would label this as a bug, as it makes development unbearable.
Commenting out the line interface Array<T> extends Ember.ArrayPrototypeExtensions<T> {}
in types/package-name/index.d.ts
"fixes" the problem, though it leads to other problems obviously...
On my to-do list for most likely next week to get a solid repro so I can help the TS team chase down the root of this regression.
I did some playing around with this over the holiday break. It seems to be related to the conditional types in UnwrapComputedPropertyGetter
. It does not seem to be the infer
(I took it out and same result) but when I change the conditionals to hard-coded types, it compiles fast.
Default types:
$ time tsc
real 0m44.633s
Remove infer
(e.g. hard-code string
instead of inferred generic U
):
$ time tsc
real 0m46.230s
Remove conditional (e.g. type UnwrapComputedPropertyGetter<T> = string;
):
$ time tsc
real 0m3.670s
I also found that compile time is dependent on the number of usages of UnwrapComputedPropertyGetter
. If I start removing usages in @types/ember__object/observable.d.ts
:
Remove get
:
$ time tsc
real 0m23.995s
Also remove getWithDefault
:
$ time tsc
real 0m9.545s
Also remove cacheFor
:
$ time tsc
real 0m3.977s
Interestingly, removing getProperties
(which uses UnwrapComputedPropertyGetters
, which uses UnwrapComputedPropertyGetter
) had very little affect on compile time:
Remove getProperties
:
$ time tsc
real 0m42.193s
That tracks with my own observation around where the regression happened in TS 3.1: it seems to be a regression when combining mapped types and conditional types.
We worked around this in ember-osf-web
(as @mogstad mentioned, but also including Copyable
).
3.1 definitely made it much worse:
$ npm install typescript@~3.1
+ [email protected]
$ time npx tsc
real 7m53.560s
but all those times above were with TS 2.8!
@danielrosenwasser just raising this for your info; I'm going to try to have a minimal repro of some sort next week filed as a bug on the TS repo on both the basic compile time issues and the major 3.1 regression; any pointers as to the best way to get the info you all need for that would be 💯.
Trying to spend a bit of time digging into this today. Will report what I find!
Minimal repro 👍
But also reporting with --extendedDiagnostics
would go farther.
@DanielRosenwasser will do! Anything else I should look for in terms of isolating where it's coming from?
Just keep deleting surrounding code until you get a reasonable build time and build back up - maybe try to see what array extensions trigger it. @sanders_n might know more.
Ok, this is bizarre! Apparently, order matters. I'll explain.
For https://github.com/mogstad/typescript-ember-slow-example
Using tsc 3.2.4, I get:
$ time npx tsc
real 9m57.583s
If I go into node_modules/@types/ember__object/observable.d.ts
and move the definitions for getProperties
and setProperties
(that use the mapped types UnwrapComputedPropertyGetters
and UnwrapComputedPropertySetters
) to be the last things in the interface (so that they are defined after everything that uses UnwrapComputedPropertyGetter
and UnwrapComputedPropertySetter
), I get:
$ time npx tsc
real 0m53.262s
I discovered this accidentally while creating a minimal repro (which I'll post shortly).
That would explain some of the difficulty I've been having rebuilding an example that explodes. It also maps with my observation that imports seem to be relevant, likely because of resolution order?
@DanielRosenwasser I've pushed a repo with a repro-of-sorts here. I'm happy to open an issue, but I'm actually not exactly sure how to capture this at this point:
- there's a baseline of not great performance on this specific combination of features
- there's a serious regression in the 3.1.x–3.2.x era (in this repo, ~5×) – maybe also 3.3.x, it was still present on 3.3.0-dev but I'm not sure about the current state
- the performance regression appears to be fixed in 3.4.0-dev.20190131
- but whatever changed so that the performance regression is resolved also broke a conditional type which worked from 2.8 forward
😬
I've created a minimal repro. I found the same as @chriskrycho: major performance regression in 3.1-3.3 that appears to be fixed in 3.4, but there are other issues with conditional types. Those issues are not reproed in my performance regression repro, however.
Would this only apply to .ts
files in the project? My build seemed to be hanging and I narrowed it down to a Promise.all(ary)
line, but it's in a .js
file. It either permanently hangs the build, or triggers a build longer than I've ever been willing to wait.
I'm pretty new to typescript - does the compiler run on js files as well?
(I've replaced it with for (let p of ary) { await p; }
as a workaround, which works fine)
We came across the same problem luckily @mnunmununm from the discord channel pointed us to this issue. Hopefully, it can be resolved someday because it's not a good developer experience when you are blocked by this. Maybe we should add a section: known issues
to the readme?
@tschoartschi: The problem seems to be solved in typescript >=3.4.1
@tschoartschi there's a Known Issues section in the docs, and I'd happily take a PR noting this is an issue on TS versions 3.1–3.3 there.
We also plan to address these kinds of issues over the next few months; there'll be an ember-cli-typescript RFC forthcoming outlining the strategy for that.
@chriskrycho you mean the https://typed-ember.github.io/ember-cli-typescript/versions/master/docs/ts-guide/current-limitations section? I could write something explaining this issue. Or maybe it's enough to just link here. To be honest, I was quite lost today and spent the whole afternoon digging into why my build was broken.
@mschorsch thanks for the pointer to TypeScript 3.4.1. I'll need to check that our tomorrow :-)
Nevertheless, it's awesome work you guys did with ember-cli-typescript
!
Closing as resolved in TS 3.4. The learnings from this fed into our design of https://www.semver-ts.org and Ember's own policy that it is not required to add any given TS version to its support matrix, as well as our ecosystem-wide policy of testing against upcoming versions of TS to catch issues like this and report it upstream!