Convert the codebase to modules #51387
Merged
+282,700
−284,842
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary |
typescript-bot
added
the
For Milestone Bug
|
@typescript-bot test this |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite (tsserver) on this PR at 4ddaab5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 4ddaab5. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the extended test suite on this PR at 4ddaab5. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 4ddaab5. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 4ddaab5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the perf test suite on this PR at 4ddaab5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 4ddaab5. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the diff-based user code test suite (tsserver) on this PR at 4ddaab5. You can monitor the build here. Update: The results are in! |
This was referenced Nov 2, 2022
jakebailey
force-pushed
the
typeformer-2
branch
from
4ddaab5
to
23272cd
Compare
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@jakebailey Here are the results of running the user test suite comparing Everything looks good! |
1 similar comment
|
@jakebailey Here are the results of running the user test suite comparing Everything looks good! |
|
Wahoo!! |
jakebailey
added
Breaking Change
|
@jakebailey Here they are:
CompilerComparison Report - main..51387
System
Hosts
Scenarios
TSServerComparison Report - main..51387
System
Hosts
Scenarios
StartupComparison Report - main..51387
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Congrats! This is very exciting. I believe this PR also closes #39247. |
|
It does! Thanks. |
See issue 10937 on github.com/github/codeql.
The troubleshooting wizard for the debugger actually says that this option is no longer recommended; removing it enables us to use source maps if we happen to be debugging while in --bundle=false mode.
It turns out that the import expression won't actally be rewritten by esbuild, so we can just write it directly. While this won't help CJS emit, that already didn't work anyway, and it's likely that this code is going to be moved outside of the codebase into VS Code or a shared package elsewhere anyway.
jakebailey
force-pushed
the
typeformer-2
branch
from
f16b741
to
da6f067
Compare
|
Alright everyone, it's happening. See you on the other side. |
|
Congratulations, Jake! 🌟 |
|
If you have a PR and are struggling to figure out how to fix your branches (after realizing what happened to main), here's what I am finding works the best:
|
4 tasks
This was referenced Nov 8, 2022
This was referenced Nov 10, 2022
This was referenced Feb 18, 2023
Merged
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is it; the PR that converts the TypeScript repo from namespaces to modules.
TL;DR: The TypeScript compiler is now implemented internally with modules, not namespaces. The compiler is now 10-25% faster.
tscis now 30% faster to start. Our npm package is now 43% smaller. More improvements are in the works.If you're reading this and the 5.0 beta is out, now that the above results were as of this PR; the final performance change of TS 5.0 is different!
Closes #35210
Closes #39247
Closes #49037
Closes #49332
Closes #50758
For #27891
For #32949
For #34617
For those who are looking at the diff and wondering how they can see what this PR actually does, consider looking at each commit, or look at my silly stacked PR gerrit-clone thing at jakebailey#1 (which I will try to keep up to date).
Want to try this out now? Set this in your
package.json:(Or, wait until the next nightly after this is merged.)
How?
The bulk of the work to convert the repo to modules is done via automation, followed by a load of changes by hand to completely restructure our build system. To see the conversion process, take a look at the commits in this PR, each of which come with their own descriptive commit message.
In short, the typeformer project recreates our old namespaces as "barrel" modules, reexporting each file from the barrel module corresponding to its original namespace. Symbols which previously were available "globally" via namespaces are now imported in big import blocks at the top of each file, in such a way that the final code looks nearly identical to the input (but unindented one level).
This, with a little hand modification, gets us to a place where we can emit CJS and have everything working.
However, this turns out to incur a big performance penalty; our old TS namespaces were just objects, but CJS imports/exports are more expensive than plain objects. Additionally, many consumers currently expect to be able to load TS as a single file, and that same file must be usable in Node and the browser. Unless we wanted to change that expectation and break the world, we needed to bundle.
As an experiment while working on this conversion, I tried simply running esbuild on our source files directly; with a little more tweaking, I was able to produce a working output for our codebase. Unexpectedly, this also brought major performance wins in our benchmarks (see the next section). Those performance benefits (and speedy build times) were too good to pass up, and so, our main JS outputs are now produced by esbuild.
To mitigate the risk of not using
tsc's output, I have created a--bundle=falsemode for our build. This mode usestscto emit CJS, producing modules matchingsrc1:1. This is tested in CI so we can reliably switch back totsc's CJS emit if needed. The new build is also constructed such that it'd be easy to transition to running a bundler ontsc's ESM emit (rather than directly onsrc).Since our outputs are bundled, we needed a way to "bundle" our d.ts files too. To handle this, I wrote a small-ish (400 line) d.ts bundler that can serve our needs. This bundler is "dumb" and cannot handle cases that a more widely-usable bundler (like api-extractor) should, e.g. properly renaming types with the same name but declared in separate files or avoiding aliasing globals, which are cases that we can avoid or ignore, respectively.
Benefits for users of TypeScript
Firstly, the performance of the compiler has increased by anywhere from 10 to 25%. This largely thanks to scope lifting. TypeScript namespaces are emitted as a series of IIFEs which push properties onto a single object. Non-local accesses to exports of that namespace are implicitly fully qualified, incurring the overhead of an object access. With modules, all of our internal helpers have been lifted to the top and are available for direct calls.
Secondly, our package is now ~43% smaller (from 63.6 MB down to 35.6 MB). This is due in part to dropping some redundant files from our package, including
typescriptServices.js, which was identical totypescript.js(see the breaking changes section below), a change in the indentation used in our bundle files (4 spaces -> 2 spaces), plus a large amount of tree shaking intsc.jsandtypingsInstaller.js. TypeScript 4.9 was made smaller by 2 MB already (thanks to #50421), so this is excellent progress.Finally, as a result of both of the previous performance improvements (faster code and less of it),
tsc.jsis 30% faster to start andtypescript.js(our public API) is 10% faster to import. As we improve performance and code size, these numbers are likely to improve. We now include these metrics in our benchmarks to track over time and in relevant PRs.Benefits for the TypeScript team (and contributors)
For those who work on TypeScript itself, this change comes with a number of important benefits.
Now that we are modules, we are dogfooding the module experience. Effectively everyone is using modules and has been for a long time, but we've been using namespaces. This left us unable to experience things like auto-imports, giant import blocks, etc, ourselves. (During testing of this PR, we found multiple interesting auto-import and quick fix bugs; win!)
Being modules also provides us the ability to start integrating excellent external tooling into our build process. As previously mentioned, our JS outputs are now produced by esbuild. This lets us start running tests in seconds (or less), representing a significant shortening of the develop/test/debug loop.1 Of course, this is potentially limiting; if we need new syntax features that aren't yet implemented in esbuild, we can switch to bundling the ESM output of
tscinstead, restoring the status quo.Additionally, since I had to change the entire build process to support this conversion, it's a great time to make build changes that would normally be difficult during our typical dev cycle. This includes replacing our task runner with a lightweight alternative (we don't need too much fanciness anymore), rewriting/deleting a number of scripts, and removing/replacing outdated dependencies. After this change, our
npm auditis clean and our dev dependency count has been halved.No, we're not yet shipping ESM
This PR does not convert the TypeScript package to ESM (#32949). The package still consists of a handful of un-tree-shakeable large files (though there are now fewer of them, and they are smaller). Allowing TypeScript to be packaged as ESM is a further challenge that will build on this change, but for this PR, the goal is to convert the repo's source files to modules without causing major breaks for downstream users. (For the future, see the "future" section below.)
Breaking changes
This conversion introduces a number of breaking changes for consumers of the TypeScript package; those using
tscor the language service in their editor should see no changes (besides improved performance and smaller install sizes).typescriptServices.jshas been removed; this file was identical totypescript.js, the entrypoint for our npm package.protocol.d.tsis no longer included in the package; usetsserverlibrary.d.ts'sts.server.protocolnamespace instead.ts.server.protocolnamespace, but were emitted in the oldprotocol.d.tsfile, and may need to be accessed off of thetsnamespace instead. See Don't depend on typescript protocol.d.ts vscode#163365 for an potential way to minimize changes to protocol-using codebases.The future?
The module conversion opens the door to many future improvements.
Our codebase is massively circular thanks to how easy it was to introduce cycles with namespaces. Namespace emit ordering was controlled by
tsconfig.json, which allowed us to structure things "just right" and not crash, but things aren't so straightforward in modules. The modules themselves define their execution order, and the current state is one that works (even if the use of "namespace barrels" is suboptimal). But, this shouldn't be permanent; now that we are modules and it's not costly to introduce new files into the mix, we can start restructuring the codebase and unraveling our import cycles. In an ESM future, breaking these cycles may lead to improved tree shaking for those who bundle TypeScript into their codebases.In terms of performance, a small number of namespaces remain in the codebase (
Debug,Parser, others) which would benefit from being converted to modules to allow for direct calls. We also lost some performance in our parser due tolet/const(disproportionally so; the other parts of the compiler were only slightly affected but had much larger net wins). We may be able to restructure the parser to avoid the temporal dead zone performance cost.3In terms of code size, may even be able to do some amount of ESM emit now. A few of our bundles are not for import, only for execution (
tsc,tsserver,typingsInstaller). If we are willing to drop versions of Node before ESM (Node <12), we can drop another 7 MB from our package by using esbuild's ESM code splitting for those bundles.In the process of making bundling work, I fixed
monacoto no longer depend on the source-level structure of our package. This means that we could potentially start minifying our codebase, reducing the package size significantly. This is not without gotchas, of course; we know that there are downstream users who patch our package post-install, which is much more difficult when the output isn't predictably formatted (or effectively impossible using "patch" when minified).On the shoulders of giants
I have many people to thank:
PR notes:
.git-blame-ignore-revsas it references specific commits in the PR thatgit blameshould ignore; I want to ignore only the automated steps in this PR. I have attempted to make this PR as few commits as possible while still making sense.srcinmainwill cause this PR to have conflicts, and the only way to fix that is to rerun the entire transformation from scratch and force push.Footnotes
Specifically, the time it takes to to run the "local" build task (everything but tests) has been reduced from ~30s to ~20s on my machine. When changing the checker, the build time has been reduced from ~18s to ~14 seconds. When changing nothing, the build time has been reduced from 1.3s to 0.7s. ↩
ES2018 is the ECMAScript version implemented by Node 10. We could have bumped to ES2019 (Node 12), but this would only give us optional catch binding; we already conditionally use newer library features like
trimStart/trimEnd. We could have also bumped up to ES2020 (Node 14, though, see Recommended tsconfig.json target option for Node 14 is ES2020 which is not fully supported by Node 14. #46325), which would give us native nullish coalescing and optional chaining, but it seemed like too big of a jump to drop Node 12, even if it is end-of-life and untested in our CI. ↩In an earlier revision of this stack, I used babel to downlevel
let/consttovarafter bundling. This increased performance as expected, but we decided to not add this step; in the ESM future, we wouldn't want to end up with all of ourexport constdeclarations turning intoexport var, and avoiding the transformation simplifies the build and improves the debugging experience when loops are involved. If we change our minds, this is easy to reimplement, albeit slow. ↩