Const enums in the TS Compiler API can make depending on typescript difficult

[weswigham]

At present, if I write a library which depends on typescript, I implicitly lock my consumers into using a typescript package version which has a SyntaxKind which matches the numbers which existed when I compiled my library, and failures resulting from a mismatch in runtime and compile-time SyntaxKinds only occur as unexpected runtime behavior (like a tslint no-null rule triggering on if statements). This behavior change may only even be noticed when the package is consumed and not when it's tested (since, frequently, you test with the same TS you build with).

I would suppose that fixing this could require either considering changes to our API's const enums as semver breaking changes to make it easier on npm, or we should stop using const enums in our API, so the name-based references are retained in consumers' compiled output. Alternatively, we could continue to use const enums internally, but in our built .d.ts we report them all as normal enums (and we compile with preserveConstEnums, as we already do), so our consumers retain the references in their built code.

[mhegazy]

Removing the "const" in post build seems reasonable. feel free to send a PR for this.

[myitcv]

Not sure whether this is related. As I mention in this comment there is a difference in typescript.d.ts on master and in the nightly builds when it comes to enums, e.g. NullKeyword:

Nightly build typescript@1.7.0-dev.20151006 defines NullKeyword = 91
Nightly build typescript@1.7.0-dev.20151014 defines NullKeyword = 93
Nightly build typescript@1.7.0-dev.20151015 defines NullKeyword = 93
Nightly build typescript@1.7.0-dev.20151016 defines NullKeyword = 93

However master (at the time of writing) defines NullKeyword = 91.

It also shows that typescript.d.ts was last updated on Sep 15.

Is this intentional?

[myitcv]

On a related note, would it be possible to include the commit reference in the nightly builds to see exactly where they were cut?

[mhegazy]

It also shows that typescript.d.ts was last updated on Sep 15.

that sounds right, this is the LKG we use to bootstrap the compiler. we update it occasionally, but not regularly.
the script that publishes the nightly ships the latest from master.

would it be possible to include the commit reference in the nightly builds to see exactly where they were cut?

@DanielRosenwasser what can we do here to make this better?

[myitcv]

the script that publishes the nightly ships the latest from master.

But that's exactly what I'm not seeing unless I'm missing something:

Nightly build typescript@1.7.0-dev.20151016 defines NullKeyword = 93
master (at the time of writing) defines NullKeyword = 91

[DanielRosenwasser]

TypeScript performs a full build/test run (jake runtests) and performs an LKG update (jake LKG) before publishing. If you perform a build, you'll see that locally you get NullKeyword = 93.

[DanielRosenwasser]

@mhegazy we could append the first 7-digits of the commit hash to the end, though @billti had a feeling that this would be annoying to type for npm users.

A compromise would be to append the hash in the tsc version string, but not the npm published version name.

[myitcv]

If you perform a build, you'll see that locally you get NullKeyword = 93

So are you saying the committed typescript.d.ts is out of date?

If so, does it make sense to add a build step that ensures the committed typescript.d.ts matches the 'output' of jake LKG, and fail the build if that's not the case?

A compromise would be to append the hash in the tsc version string, but not the npm published version name.

Sounds good to me.

[jbondc]

I'd prefer having two files:

• An optimized build of TS (typescript.od.ts), with const enums
• A definition build of TS (typescript.d.ts)

The optimized build of TS should not be used as an external dependency. Related to:
#4150

[DanielRosenwasser]

@myitcv the idea is that whatever is in the LKG folder is the last stable version of the compiler that's able to compile the compiler. It's purely for bootstrapping when necessary/convenient and for publishing. I don't think we necessarily need to add that to the build step.

[mhegazy]

Should be fixed by #5422