typed-ember / ember-cli-typescript

Use TypeScript in your Ember.js apps!
https://docs.ember-cli-typescript.com
MIT License
363 stars 99 forks source link

Upgrading from v1.0.x to v1.1.x causes build to stall on Windows #150

Closed jacobq closed 6 years ago

jacobq commented 6 years ago

I'm not sure why this is happening, but I spent the better part of today tracing the problem to ember-cli-typescript, so hopefully others who encounter this problem can find these notes.

Symptom: running ember build (as well as ember s or ember electron, maybe others) on Windows 10 (tried with various versions of node 6.x and 8.x) does not finish. It gets stuck at "File change detected. Starting incremental compilation...". (Actually, if you run DEBUG=ember-cli-typescript:* ember build you will see something like ember-cli-typescript:compiler waiting for tsc output {} after that.) However, if you generate new app on the same system and ember install ember-cli-typescript it builds OK.

The key to solving the problem is evidently that tsconfig.json needs to be updated (I would have thought this would constitute a breaking change and so have merited a major version number bump.).

Steps to reproduce:

I realize that there are notes saying to re-run the generators when updating, so if this behavior is not a bug then feel free to close, but it would be nice if the addon just threw an error rather than hung the build process.

chriskrycho commented 6 years ago
  1. Thank you so much for the detailed report!
  2. I'm super sorry that happened that way. 😞 I've spent my own long days chasing down things like this and they're incredibly frustrating.

A quick explanation for why a change to the tsconfig.json didn't constitute a breaking change to my mind: a number of things in the Ember ecosystem have gone through changes that were in the strictest sense backwards-incompatible, but which had config-file-only level changes that solved them, and which could be prompted for by running the generators. (The baseUrl -> rootUrl change is the most obvious example I recall.)

Since via ember itself prompts with the required changes that will make your app continue to work, it seemed like a "non-breaking" change. And, along those same lines, if we did a major version bump for every time we had to tweak the tsconfig.json, we'd do so on a number of things that we've released as bugfixes!

This is also why the recommended upgrade process is not yarn upgrade ember-cli-typescript@latest but specifically calls out ember install – so that the generators get run and these things as much as possible get taken care of.

In terms of throwing an error: if you can find a way to make it throw in this circumstance, that would be awesome! I'm not sure we can, because we don't have the level of understanding from the addon's point-of-view to know whether it's hanging or not—we're just waiting for TypeScript to get back to us.

Both of these ultimately come down to the fact that we're trying to wrap the TypeScript compiler in a way that makes it mostly "Just Work" for consumers and plays nicely with Ember CLI (and props to @dwickern and @dfreeman for making it work as well as it does)—but TS does not make it easy, esp. because it's incredibly particular about exactly what combination of settings in the tsconfig.json will give the desired results. 😬

That said, I want to reiterate that the experience you had was super frustrating, and it's something we'd like to avoid in the future. 😖 We've started talking about doing more active pre-release notices and asking people to test with their existing apps so that we can catch these things before they ship rather than after. (We've done some of that, but our existing install base was pretty small, and I didn't "message" it loudly enough as we led up to 1.1.0!)

I'm going to close this because running the generators does generally solve this, but please feel free to continue the conversation in this thread.

jacobq commented 6 years ago

Thanks for the explanation @chriskrycho I guess what irk'd me, aside from being hard to track down, is that it appears to (looking back at the 9/22/17 commit) have originally installed as "ember-cli-typescript": "^1.0.3", and since I was not using yarn or package-lock.json the upgrade was able to happen without my knowing it. Perhaps it would be better, if possible, for it to ember install with its version pinned (is that possible/easy?).

As for making it fail more loudly, would you accept a PR that used something like a 5 minute timeout timer near here? It could just emit a message to the console or, if desired, could be used with Promise.race to make the returned promise reject.

chriskrycho commented 6 years ago

@jacobq – in terms of setting a long timeout there, that seems reasonable to me at first blush. Thoughts, @dfreeman and @dwickern?

Re: not having package-lock.json or yarn.lock— I basically take those as necessary givens for dealing with these kinds of things. :grimacing: At a minimum, my view is that people either should use one of those or they should specify a specific version, effectively pinning themselves to it. (I note that ember install ember-cli-typescript installed with the ^ constraint.)

If you think it would be helpful, I’d be up for a PR adding a note in the README explaining some of this (though I don’t have time to put it together myself in the near term).

dwickern commented 6 years ago

Even if we did pin our dependency versions, it wouldn't pin our dependencies' dependency versions. If we kept going and pinned all the versions transitively, well, we'd reinvent lockfiles. You absolutely do need a yarn.lock/package-lock.json! Otherwise your app can-- will break at any moment. Even if you have all your direct dependencies pinned!

To prevent this from happening again,