Broken compile with TypeScript 3.4

[linde12]

Initiative / goal

Clarity regarding versioning.

Background

Today our builds broke because we are using lit-element@^2.2.0 combined with Angular 8 (using typescript 3.4.x). I debugged for a while and found that this happens because In v2.3.0 of lit-element the generated typescript definitions were generated with typescript 3.8.x and that version is incompatible with 3.4.x. I was assuming lit-element was following semver but after a closer look i couldn't find any documentation on it. The point of this PR is just to make it clearer that we may have breaking changes when the minor flag changes.

Scope

Clarify, in README.md possibly, on what versioning strategy lit-element is following.

[justinfagnani]

Sorry you've experienced a break. Let's narrow this down to the specific break, rather than the very general title it has now. We do strive to follow semver, so I'd like to find out what actually happened.

In general, the version of TypeScript that we use shouldn't matter much for consumers, unless TypeScript itself has new backwards-incompatible feature in .d.ts output. I'm going to try run a diff between the .d.ts files, but do you have any details on what broke?

[rictic]

Workaround: try setting "skipLibCheck": true in compiler options

IMO this is not a breaking change, or a violation of semver. The code works at runtime.

The TypeScript compiler does not follow semver, so if we treated type checking issues as part of the semver contract then we would have to freeze a specific version of typescript as the only supported version until we next did a major breaking change. This wouldn't be helpful for our users, as most would, in practice, be either unable to use that version of TypeScript or would be missing out on newer TypeScript features.

That isn't to say that this isn't an issue for you that we'd like to fix – and odds are decent that it will be a pretty easy fix – it's just that we need to set expectations. This isn't unique to LitElement either, it's a general problem of including types with a library on npm.

[justinfagnani]

Reference for the TypeScript semver bug: microsoft/TypeScript#14116

[rictic]

What're the error messages you're seeing?

Poking around with a local version of TS 3.4 I'm seeing An accessor cannot be declared in an ambient context.. It looks like a recent version of tsc started emitting getters in .d.ts files, while previously they would be emitted as readonly fields.

[rictic]

Looks like we can test with other versions of TypeScript by doing:

npm ci
npm run build # build with version from npm ci
npm install typescript@~3.4
npx tsc lit-element.d.ts --noEmit -t esnext --moduleResolution Node

[rictic]

The only thing stopping us from LitElement itself building with TypeScript 3.4 is the ?. syntax, but that code expands out a decent amount when transpiled to ≤ES2019 so maybe shouldn't get in the habit of using it in LitElement or lit-html yet.

[justinfagnani]

Ideally it would be nice if TypeScript versioned the input syntax and output declaration syntax separately. We would like to use as new of features as possible for our code, but make our declaration files as compatible as possible.

[mzeiher]

this also broke our builds, same for lit-html 1.2.0, we also got into this error with our own libraries, at the moment we use https://github.com/sandersn/downlevel-dts to ship different versions of type definitions for our library maybe this would also be a way for lit-* libraries? So we can still use the newest typescript but still support old typescript versions in the d.ts files next to the current one

[justinfagnani]

Thanks for the pointer to that project @mzeiher!

I filed microsoft/TypeScript#37478 on TypeScript for this issue.

[linde12]

Sorry you've experienced a break. Let's narrow this down to the specific break, rather than the very general title it has now. We do strive to follow semver, so I'd like to find out what actually happened.

In general, the version of TypeScript that we use shouldn't matter much for consumers, unless TypeScript itself has new backwards-incompatible feature in .d.ts output. I'm going to try run a diff between the .d.ts files, but do you have any details on what broke?

No worries at all! As for the generic title I just assumed it was intentional (typescript team does this as well where they don't follow semver and instead do breaking changes on the minor flag)

As @rictic mentioned the problems lies with backwards-compat in d.ts files and the specific error i got was the "An accessor cannot be declared in an ambient context" one because of the getters in the newer version of TS.

Anyway, to prevent future confusion it might be a good idea to mention something about following semver in the readme.

[linde12]

Workaround: try setting "skipLibCheck": true in compiler options

IMO this is not a breaking change, or a violation of semver. The code works at runtime.

The TypeScript compiler does not follow semver, so if we treated type checking issues as part of the semver contract then we would have to freeze a specific version of typescript as the only supported version until we next did a major breaking change. This wouldn't be helpful for our users, as most would, in practice, be either unable to use that version of TypeScript or would be missing out on newer TypeScript features.

That isn't to say that this isn't an issue for you that we'd like to fix – and odds are decent that it will be a pretty easy fix – it's just that we need to set expectations. This isn't unique to LitElement either, it's a general problem of including types with a library on npm.

Yes, we considered using skipLibCheck but instead, as we "owned" that part of the code, we were able to just use v2.2.0 of lit-element.

IMO it is a breaking change per definition regardless if it's a dependency that is causing the break or not. It is breaking builds for some users who use an older version of typescript in their build process.

I agree that this is a problem for packages in general and i do hope the typescript team will address this at some point, but as a short term solution maybe downlevel-dts could solve the issue?

[rictic]

Yep, we should have a fix out shortly, and I'm adding a test that the downleveled .d.ts files compile in TS 3.4 to our continuous integration.

Given TypeScript's versioning and breakages policy, it's not impossible that skipLibCheck would be required at some points along the line, but with downlevel-dts we've at least got a fighting chance :)

Thanks again for the pointer @mzeiher!

[mzeiher]

glad to be able to help :), this also led to a lot of headaches within our libraries :(

[GCour]

What was the error? I'm getting this at the moment:

ERROR in node_modules/lit-element/lib/css-tag.d.ts:16:9 - error TS1086: An accessor cannot be declared in an ambient context.

16     get styleSheet(): CSSStyleSheet | null;
           ~~~~~~~~~~
node_modules/lit-element/lib/updating-element.d.ts:153:16 - error TS1086: An accessor cannot be declared in an ambient context.

153     static get observedAttributes(): string[];
                   ~~~~~~~~~~~~~~~~~~
node_modules/lit-element/lib/updating-element.d.ts:335:17 - error TS1086: An accessor cannot be declared in an ambient context.

335     private get _hasRequestedUpdate();
                    ~~~~~~~~~~~~~~~~~~~
node_modules/lit-element/lib/updating-element.d.ts:336:19 - error TS1086: An accessor cannot be declared in an ambient context.

336     protected get hasUpdated(): number;
                      ~~~~~~~~~~
node_modules/lit-element/lib/updating-element.d.ts:370:9 - error TS1086: An accessor cannot be declared in an ambient context

[rictic]

Yep, that's the failure that should be fixed by #936 @GCour