Discussion: strictness settings, TS support, semver, etc.

[chriskrycho]

(Consider this sort of pre-RFC discussion as I work on a full RFC for official TS support policies in Ember proper.)

Background

As I've been thinking about the most effective setup for using this, and looking at our current documentation, I've been chewing on this recommendation I wrote quite a few months ago now:

Start with the least strict "strictness" settings (which is what the default compiler options are set to) to begin, and only tighten them down iteratively.

I… don't think that's right. I think we should tell people that they'll be most successful with the strictest strictness and that they should start there, because progressively typing an app will mostly be a one-time pass, rather than a multi-phase process where you add stricter rules. 🤔

As part of that, I'd like to do two things:

1. Update this recommendation to say basically the opposite of what it does now, and as part of that include a code sample with the strictest settings we can support with our current typings.

   This part seems pretty uncontroversial to me; people are of course free to simply ignore the recommendation! As such, I've gone ahead and added that update to the README.

2. Update the default template to include every strictness setting.

   This part is a bit stickier! And it got me thinking about a lot of related issues, some of which we've discussed before in Slack, but which I'd like to nail down here.

The meat of the issue

The thing is, doing (2) is essentially a breaking change: if someone runs ember install ember-cli-typescript@latest and sees the diff and accepts it, a previously-type-checking app will no longer type-check. This means that making the change essentially requires us to do a 2.0. This presumably needs to include things around tsconfig.json in general: I'd also like to change the default noEmitOnError settings to true now that we're able to properly fail the build in that case, but that will also break existing apps, and so also demands a 2.0.

I'm fine with that; I don't mind doing "major" version changes in a semver sense relatively often. We just need to establish what semver means for this addon, and document it.

But this gets into broader questions around TS (and especially strictness) support we should resolve as well: should we default to shipping every new strictness setting enabled (assuming it doesn't break our typings)? Or should we ship a minimal set and just make recommendations?

One closely related issue is supported TS version in general, both here and in our type definitions. We shouldn't ship breaking changes in our type defs every 2 months when the TS compiler comes out, but we do need to make sure this addon tracks closely with the TS version.

• We could start making the ember-cli-typescript version track the version of TS it ships. I.e. right now we'd go ahead and bump this to be ember-cli-typescript 2.8, and at the next TS release we'd do ember-cli-typescript 2.9, and so on. I don't love this idea, but it's an option.

• We can just clearly document what versions of TS we know we work with – if we do this, we should probably figure out a story to make sure we actually work with the TS versions we claim; that essentially expands our test matrix exponentially with every combination of supported TS and Ember version. My thought here is that we should pick a very small number of TS versions we check – probably just most recent 2 versions (so right now 2.7 and 2.8).

At this point, though, everything here is just my opinions, though, and I'm very interested in the rest of the Typed Ember team's (@dfreeman, @dwickern, and @jamescdavis) takes, as well as broader community input.

[dfreeman]

I think we should tell people that they'll be most successful with the strictest strictness and that they should start there

I'm 100% on board with changing our recommendation and default templates to be as strict as possible. I agree with the reasoning that it ultimately leads to a smoother adoption experience, and it has the potential to show value faster as well.

We just need to establish what semver means for this addon, and document it.

As an addon consumer, I don't think I would expect a change in blueprint output that produces different behavior to require a major version bump. When I run ember g <addon-name> after upgrading an addon, I generally expect to use the result of that as a guide, but more often than not I know it will break my app if I just commit it directly. If I run ember g ember-cli-mirage, for instance, that will erase all my configured Mirage routes if I blindly accept the change.

Given that, I think I'd be in favor of generally treating changes to the default config we produce as non-breaking, and carefully documenting things like increased strictness in the changelog along the lines of:

As of e-c-ts x.y.z we now turn on `strictFooBar` by default. We recommend adopting
this setting because it catches errors like a, b and c, but if the migration cost in
your project is high or you otherwise don't want this check, you can turn it back off
in your `tsconfig.json`.

Historically, if Slack and Github are any indication, people seem to be fairly likely not to bother regenerating their config at all when they upgrade anyway 😜

One closely related issue is supported TS version in general, both here and in our type definitions.

I'd definitely lean more toward just documenting version compatibility over trying to keep lockstep versioning with TS. I suspect we could get away with hitting fewer versions of Ember in our test matrix, since ember-source itself currently doesn't actually interact with us very much at all. That might make adding TypeScript itself to the matrix more palatable.

[jamescdavis]

My thoughts are very much in line with Dan's. I just gave a TS + Ember talk to another Ember team at my org on Friday and I highly recommend starting with strictest settings. They are just starting a greenfield project (jealous), but I'd say the same for a conversion. Regarding semver, I only think we need to bump major when the API changes in a backwards incompatible way. I'm not sure exactly what that would be, but maybe something like requiring the next major TS version? Minor vers are additive. This would be just adding additional functionality (stricter checks by default). You aren't required to use those settings to use e-c-ts. And, yeah, who re-runs the blueprint every upgrade anyways? 😉 As far as lockstep versioning with TS: nope, no way, don't do it. At COS we tried and failed miserably at this with our own packages, much less a vendor package.

my 2¢

[jamescdavis]

Oh and +1 for documenting this well in the changelog, but also the readme as that, I'm sure, gets way more views.

[chriskrycho]

Given this discussion, I’m inclined to go ahead and tighten down the strictness settings, and simply adopt an explicit “always document it in the changelog and README” policy whenever we make changes. We should also link the changelog from the README; that would increase its discoverability quite a bit I think.

As for TS – glad we’re on the same page here; lockstep will be not bueno. We should just figure out what minimum version we currently support to start, and document it.

[dfreeman]

Turns out our current minimum supported version (for ember-cli-typescript, at least) is 2.7 — that's the earliest release that has the watch API.

Relatedly, I'd like to start a conversation about overhauling our current test matrix. Right now, it looks like this:

    - EMBER_TRY_SCENARIO=ember-lts-2.12
    - EMBER_TRY_SCENARIO=ember-lts-2.16
    - EMBER_TRY_SCENARIO=ember-release
    - EMBER_TRY_SCENARIO=ember-beta
    - EMBER_TRY_SCENARIO=ember-canary
    - EMBER_TRY_SCENARIO=ember-default
    - EMBER_TRY_SCENARIO=integrated-node-tests

IMO ember-source itself is probably the dependency we need to be concerned least with, since we don't actually interact with it at all. We essentially have two test suites (ember test and yarn nodetest), and I'd love to see us start running both against multiple versions of TypeScript.

I vaguely recall someone suggesting "last two releases" as a potential reasonable support range, though I'd also advocate for including typescript@next in there so we can get a heads up when something breaking (like #209) is coming down the pipe.

[dfreeman]

We might also want to consider including multiple versions of ember-cli in our testing (at least @latest and @beta) for similar reasons.

[chriskrycho]

Concur. ember-source should be covered by tests in Definitely Typed.