[DO NOT REVIEW] More module-ified compiler testing #51144
Closed
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
This step makes further commits look clearer by unindenting all of the top level namespaces preemptively.
This step makes all implicit namespace accesses explicit, e.g. "Node" turns into "ts.Node".
This step converts each file into an exported module by hoisting the namespace bodies into the global scope and transferring internal markers down onto declarations as needed. The namespaces are reconstructed as "barrel"-style modules, which are identical to the old namespace objects in structure. These reconstructed namespaces are then imported in the newly module-ified files, making existing expressions like "ts." valid.
This step converts as many explicit accesses as possible in favor of direct imports from the modules in which things were declared. This restores the code (as much as possible) back to how it looked originally before the explicitify step, e.g. instead of "ts.Node" and "ts.Symbol", we have just "Node" and "Symbol".
Now that we are modules, there's no reason to ban multiple namespaces per file; each file is its own scope and declaring a namespace won't merge it into any other files.
If these are regular comments, then they won't appear in our d.ts files. But, now we are relying on api-extractor to produce out final merged d.ts files, so they need to be present in the "input" d.ts files, meaning they have to be JSDoc comments. These comments only work today because all of our builds load their TS files from scratch, so they see the actual source files and their non-JSDoc comments. The comments also need to be attached to a declaration, not floating, otherwise they won't be used by api-extractor, so move them if needed.
This project is the same as the (soon added) typescript project.
This file is unreferenced, and just a subset of tsserverlibrary.d.ts. What I've heard is that this was intended to be used to detect protocol chagnes, but, no test actually references it, and we have separate infrastructure in place to ensure that protocol changes get the attention they need.
This configures the existing build tasks to use esbuild by defualt. If using the plain files is desired, passing `--bundle=false` will build using plain files and still produce a runnable system.
Profiling the build roughly half of the time spent loading the build is spent importing typescript.js, for this one function. Since this stack is already adding required devDependencies, switch readJson to use jsonc-parser (published by the VS Code team), rather than importing the entire LKG typescript.js library.
The next commit will switch the build system, but git doesn't detect that the two config files are related.
typescript-bot
added
Author: Team
For Uncommitted Bug
|
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 pack this |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at d5f2ec7. You can monitor the build here. |
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running |
This eliminates a significant number of dependencies, eliminating all npm audit issues, speeding up `npm ci` by 20%, and overall making the build faster (faster startup, direct code is faster than streams, etc) and clearer to understand. I'm finding it much easier to make build changes for the module transform with this; I can more clearly indicate task dependencies and prevent running tasks that don't need to be run. Given we're changing our build process entirely (new deps, new steps), it seems like this is a good time to change things up.
The old gulpfile produced lib.d.ts files with mixed newlines, thanks to the files containing CRLF, but the gulp streams adding in LF. Now they're all LF, which matches every other file in built / lib, but our baselines are sensitive to this. Rerun the tests and accept them.
needsUpdate may be wrong when the branch changes; these ones are now so fast thanks to being pure JS that we can just always run their contents and be sure that the outputs are right.
This file is pretty much the same as it was when it was committed in 2017; these days, we can write clearer code with async/await and new FS APIs.
Switching to a newer/faster/maintained library speeds up this script by more than 50%. We can also eliminate some any and such in the process.
If our build scripts target Node 10+, we can use the builtin mkdir function in recursive mode instead.
jakebailey
force-pushed
the
typeformer-2
branch
2 times, most recently
from
1203db7
to
1547337
Compare
|
@typescript-bot pack this |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 1547337. You can monitor the build here. |
|
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
force-pushed
the
typeformer-2
branch
from
1547337
to
157a893
Compare
However, this script appears to not work as designed; the playground instead requests tsWorker.js, a file that we can't produce in our repo. Probably, we should just remove this script.
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.
Run so I can test microsoft/monaco-editor#3356.