Repository navigation
Const enums in the TS Compiler API can make depending on typescript difficult #5219
Description
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.
Activity
mhegazy commentedon Oct 12, 2015
Removing the "const" in post build seems reasonable. feel free to send a PR for this.
myitcv commentedon Oct 16, 2015
Not sure whether this is related. As I mention in this comment there is a difference in
typescript.d.tsonmasterand in the nightly builds when it comes to enums, e.g.NullKeyword:Nightly build
typescript@1.7.0-dev.20151006definesNullKeyword = 91Nightly build
typescript@1.7.0-dev.20151014definesNullKeyword = 93Nightly build
typescript@1.7.0-dev.20151015definesNullKeyword = 93Nightly build
typescript@1.7.0-dev.20151016definesNullKeyword = 93However
master(at the time of writing) definesNullKeyword = 91.It also shows that
typescript.d.tswas last updated on Sep 15.Is this intentional?
myitcv commentedon Oct 16, 2015
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 commentedon Oct 16, 2015
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.
Daniel Rosenwasser (@DanielRosenwasser) what can we do here to make this better?
myitcv commentedon Oct 16, 2015
But that's exactly what I'm not seeing unless I'm missing something:
Nightly build
typescript@1.7.0-dev.20151016definesNullKeyword = 93master(at the time of writing) definesNullKeyword = 91DanielRosenwasser commentedon Oct 16, 2015
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 getNullKeyword = 93.DanielRosenwasser commentedon Oct 16, 2015
Mohamed Hegazy (@mhegazy) we could append the first 7-digits of the commit hash to the end, though Bill Ticehurst (@billti) had a feeling that this would be annoying to type for npm users.
A compromise would be to append the hash in the
tscversion string, but not the npm published version name.myitcv commentedon Oct 16, 2015
So are you saying the committed
typescript.d.tsis out of date?If so, does it make sense to add a build step that ensures the committed
typescript.d.tsmatches the 'output' ofjake LKG, and fail the build if that's not the case?Sounds good to me.
jbondc commentedon Oct 18, 2015
I'd prefer having two files:
optimizedbuild of TS (typescript.od.ts), with const enumsdefinitionbuild of TS (typescript.d.ts)The
optimizedbuild of TS should not be used as anexternal dependency. Related to:#4150
DanielRosenwasser commentedon Oct 18, 2015
Paul Jolly (@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 commentedon Oct 29, 2015
Should be fixed by #5422