Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable --isolatedDeclarations on TS codebase #59635

Merged
merged 28 commits into from
Sep 16, 2024

Conversation

iisaduan
Copy link
Member

@iisaduan iisaduan commented Aug 14, 2024

fixes #59097 : enables --isolatedDeclarations on our codebase

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Aug 14, 2024
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

Copy link
Member Author

@iisaduan iisaduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the generator types: for 5.5, I had to use any in these cases since unknown is not assignable to undefined (and the inferred TNext type in these cases is undefined

src/compiler/core.ts Show resolved Hide resolved
src/compiler/core.ts Show resolved Hide resolved
src/compiler/core.ts Outdated Show resolved Hide resolved
src/compiler/core.ts Show resolved Hide resolved
src/compiler/core.ts Show resolved Hide resolved
src/harness/collectionsImpl.ts Outdated Show resolved Hide resolved
src/harness/collectionsImpl.ts Outdated Show resolved Hide resolved
src/harness/collectionsImpl.ts Show resolved Hide resolved
src/harness/collectionsImpl.ts Show resolved Hide resolved
src/compiler/core.ts Show resolved Hide resolved
src/compiler/core.ts Outdated Show resolved Hide resolved
Copy link
Member Author

@iisaduan iisaduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are updates to as const objects (and other related kinds of objects)

and this #59635 (comment)

src/harness/vfsUtil.ts Show resolved Hide resolved
src/compiler/types.ts Show resolved Hide resolved
src/compiler/types.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Show resolved Hide resolved
src/compiler/utilitiesPublic.ts Show resolved Hide resolved
scripts/processDiagnosticMessages.mjs Show resolved Hide resolved
scripts/processDiagnosticMessages.mjs Show resolved Hide resolved
src/compiler/builder.ts Outdated Show resolved Hide resolved
src/compiler/debug.ts Outdated Show resolved Hide resolved
@DanielRosenwasser DanielRosenwasser changed the title turn on --isolated declarations Enable --isolatedDeclarations on TS codebase Aug 14, 2024
Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty hard to review the whole thing because the diff is so big, but I do wonder if we should even bother doing this for harness/testRunner given those don't actually have a public API...

In general though I do think we should try and name some of the types introduced here. That and there are some really unfortunate losses of inference here :(

src/compiler/expressionToTypeNode.ts Outdated Show resolved Hide resolved
src/compiler/moduleNameResolver.ts Outdated Show resolved Hide resolved
src/compiler/sourcemap.ts Outdated Show resolved Hide resolved
src/compiler/sys.ts Outdated Show resolved Hide resolved
src/compiler/transformers/declarations.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Show resolved Hide resolved
src/compiler/utilities.ts Show resolved Hide resolved
src/compiler/utilities.ts Outdated Show resolved Hide resolved
src/compiler/utilities.ts Show resolved Hide resolved
src/server/editorServices.ts Show resolved Hide resolved
Copy link
Member Author

@iisaduan iisaduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are the places where any shows up (not counting workarounds for generators). there could be better ways to type them.

src/compiler/utilities.ts Show resolved Hide resolved
src/harness/evaluatorImpl.ts Show resolved Hide resolved
src/harness/evaluatorImpl.ts Show resolved Hide resolved
src/server/session.ts Show resolved Hide resolved
@iisaduan
Copy link
Member Author

@typescript-bot perf test this faster

@iisaduan
Copy link
Member Author

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 13, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
perf test this faster ✅ Started 👀 Results

src/compiler/builder.ts Outdated Show resolved Hide resolved
@typescript-bot
Copy link
Collaborator

@iisaduan
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,242 ~ ~ ~ p=1.000 n=6
Memory used 193,618k (± 0.98%) 193,675k (± 0.92%) ~ 192,412k 196,002k p=0.298 n=6
Parse Time 1.30s (± 0.64%) 1.30s (± 1.57%) ~ 1.27s 1.33s p=0.797 n=6
Bind Time 0.71s 0.71s ~ ~ ~ p=1.000 n=6
Check Time 9.54s (± 0.41%) 9.55s (± 0.38%) ~ 9.49s 9.59s p=0.573 n=6
Emit Time 2.71s (± 0.66%) 2.71s (± 1.97%) ~ 2.61s 2.76s p=0.371 n=6
Total Time 14.27s (± 0.29%) 14.28s (± 0.49%) ~ 14.15s 14.34s p=0.520 n=6
angular-1 - node (v18.15.0, x64)
Errors 7 7 ~ ~ ~ p=1.000 n=6
Symbols 945,753 945,753 ~ ~ ~ p=1.000 n=6
Types 410,067 410,067 ~ ~ ~ p=1.000 n=6
Memory used 1,222,749k (± 0.00%) 1,222,727k (± 0.00%) ~ 1,222,642k 1,222,798k p=0.298 n=6
Parse Time 6.66s (± 0.30%) 6.65s (± 0.70%) ~ 6.59s 6.73s p=0.625 n=6
Bind Time 1.86s (± 0.22%) 1.86s (± 0.40%) ~ 1.85s 1.87s p=0.389 n=6
Check Time 31.20s (± 0.33%) 31.27s (± 0.25%) ~ 31.16s 31.34s p=0.296 n=6
Emit Time 15.10s (± 0.47%) 15.06s (± 0.15%) ~ 15.02s 15.08s p=0.517 n=6
Total Time 54.81s (± 0.29%) 54.84s (± 0.18%) ~ 54.73s 54.99s p=0.687 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,529,234 2,529,234 ~ ~ ~ p=1.000 n=6
Types 935,840 935,840 ~ ~ ~ p=1.000 n=6
Memory used 2,364,190k (± 0.00%) 2,364,164k (± 0.00%) -26k (- 0.00%) 2,364,126k 2,364,184k p=0.031 n=6
Parse Time 9.42s (± 0.42%) 9.40s (± 0.26%) ~ 9.37s 9.42s p=0.225 n=6
Bind Time 2.17s (± 0.63%) 2.18s (± 0.65%) ~ 2.16s 2.20s p=0.623 n=6
Check Time 73.39s (± 0.46%) 73.05s (± 0.47%) ~ 72.63s 73.60s p=0.093 n=6
Emit Time 0.28s (± 3.49%) 0.28s (± 1.45%) ~ 0.28s 0.29s p=0.753 n=6
Total Time 85.26s (± 0.41%) 84.91s (± 0.40%) ~ 84.51s 85.44s p=0.093 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,250,418 1,247,568 -2,850 (- 0.23%) ~ ~ p=0.001 n=6
Types 265,229 264,308 -921 (- 0.35%) ~ ~ p=0.001 n=6
Memory used 2,402,888k (± 0.01%) 2,399,829k (± 0.02%) -3,059k (- 0.13%) 2,398,957k 2,400,333k p=0.005 n=6
Parse Time 5.11s (± 1.12%) 5.06s (± 0.55%) ~ 5.02s 5.09s p=0.149 n=6
Bind Time 1.90s (± 0.61%) 1.91s (± 0.43%) ~ 1.90s 1.92s p=0.401 n=6
Check Time 34.88s (± 0.39%) 34.77s (± 0.18%) ~ 34.67s 34.83s p=0.230 n=6
Emit Time 3.32s (± 1.41%) 3.03s (± 3.37%) 🟩-0.29s (- 8.59%) 2.93s 3.21s p=0.005 n=6
Total Time 45.21s (± 0.21%) 44.79s (± 0.20%) -0.42s (- 0.94%) 44.68s 44.93s p=0.005 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,250,418 1,247,568 -2,850 (- 0.23%) ~ ~ p=0.001 n=6
Types 265,229 264,308 -921 (- 0.35%) ~ ~ p=0.001 n=6
Memory used 2,477,458k (± 0.03%) 2,474,443k (± 0.03%) -3,015k (- 0.12%) 2,473,585k 2,475,438k p=0.005 n=6
Parse Time 6.31s (± 1.05%) 6.32s (± 0.43%) ~ 6.29s 6.35s p=0.375 n=6
Bind Time 2.04s (± 0.72%) 2.05s (± 0.40%) ~ 2.04s 2.06s p=0.254 n=6
Check Time 41.39s (± 1.00%) 41.71s (± 0.75%) ~ 41.26s 42.03s p=0.148 n=6
Emit Time 4.02s (± 2.19%) 3.57s (± 1.36%) 🟩-0.45s (-11.23%) 3.50s 3.61s p=0.005 n=6
Total Time 53.78s (± 0.71%) 53.67s (± 0.59%) ~ 53.19s 53.99s p=0.810 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 259,449 259,700 +251 (+ 0.10%) ~ ~ p=0.001 n=6
Types 105,986 106,182 +196 (+ 0.18%) ~ ~ p=0.001 n=6
Memory used 433,875k (± 0.01%) 435,718k (± 0.03%) +1,843k (+ 0.42%) 435,569k 435,892k p=0.005 n=6
Parse Time 3.44s (± 0.85%) 3.43s (± 0.44%) ~ 3.40s 3.44s p=0.506 n=6
Bind Time 1.28s (± 0.32%) 1.30s (± 0.31%) +0.02s (+ 1.30%) 1.29s 1.30s p=0.003 n=6
Check Time 18.12s (± 0.51%) 18.13s (± 0.40%) ~ 18.03s 18.20s p=0.808 n=6
Emit Time 1.65s (± 1.31%) 1.53s (± 1.67%) 🟩-0.12s (- 7.19%) 1.51s 1.57s p=0.005 n=6
Total Time 24.48s (± 0.44%) 24.39s (± 0.35%) ~ 24.27s 24.51s p=0.199 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 68 68 ~ ~ ~ p=1.000 n=6
Symbols 225,018 225,018 ~ ~ ~ p=1.000 n=6
Types 94,249 94,249 ~ ~ ~ p=1.000 n=6
Memory used 370,227k (± 0.02%) 370,217k (± 0.02%) ~ 370,122k 370,291k p=0.936 n=6
Parse Time 2.77s (± 0.59%) 2.77s (± 0.99%) ~ 2.73s 2.81s p=0.871 n=6
Bind Time 1.57s (± 0.52%) 1.57s (± 0.63%) ~ 1.55s 1.58s p=0.604 n=6
Check Time 15.76s (± 0.32%) 15.74s (± 0.14%) ~ 15.71s 15.77s p=0.628 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.10s (± 0.25%) 20.09s (± 0.23%) ~ 20.01s 20.13s p=0.809 n=6
vscode - node (v18.15.0, x64)
Errors 1 1 ~ ~ ~ p=1.000 n=6
Symbols 3,059,799 3,059,799 ~ ~ ~ p=1.000 n=6
Types 1,057,277 1,057,277 ~ ~ ~ p=1.000 n=6
Memory used 3,166,989k (± 0.00%) 3,166,958k (± 0.00%) ~ 3,166,891k 3,167,002k p=0.298 n=6
Parse Time 13.91s (± 0.25%) 13.91s (± 0.43%) ~ 13.85s 14.02s p=0.469 n=6
Bind Time 4.35s (± 0.45%) 4.34s (± 0.32%) ~ 4.33s 4.36s p=1.000 n=6
Check Time 81.76s (± 0.40%) 81.77s (± 0.25%) ~ 81.37s 81.94s p=1.000 n=6
Emit Time 22.19s (± 0.37%) 22.23s (± 0.61%) ~ 22.04s 22.43s p=0.521 n=6
Total Time 122.20s (± 0.24%) 122.25s (± 0.21%) ~ 121.81s 122.50s p=0.873 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 277,156 277,156 ~ ~ ~ p=1.000 n=6
Types 112,946 112,946 ~ ~ ~ p=1.000 n=6
Memory used 426,970k (± 0.02%) 427,020k (± 0.02%) ~ 426,934k 427,156k p=0.471 n=6
Parse Time 4.90s (± 0.44%) 4.90s (± 0.45%) ~ 4.87s 4.93s p=0.870 n=6
Bind Time 2.15s (± 0.70%) 2.13s (± 0.81%) ~ 2.10s 2.15s p=0.210 n=6
Check Time 21.80s (± 0.43%) 21.81s (± 0.23%) ~ 21.73s 21.85s p=1.000 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 28.85s (± 0.37%) 28.84s (± 0.21%) ~ 28.76s 28.92s p=0.872 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 539,317 539,317 ~ ~ ~ p=1.000 n=6
Types 178,997 178,997 ~ ~ ~ p=1.000 n=6
Memory used 484,151k (± 0.02%) 484,230k (± 0.04%) ~ 483,968k 484,507k p=0.471 n=6
Parse Time 3.42s (± 0.40%) 3.38s (± 0.39%) -0.04s (- 1.12%) 3.36s 3.39s p=0.005 n=6
Bind Time 1.25s (± 0.71%) 1.26s (± 0.32%) ~ 1.25s 1.26s p=0.086 n=6
Check Time 18.30s (± 0.45%) 18.31s (± 0.27%) ~ 18.22s 18.36s p=0.936 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.97s (± 0.36%) 22.94s (± 0.26%) ~ 22.84s 23.01s p=0.629 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

src/compiler/scanner.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Show resolved Hide resolved
src/compiler/types.ts Outdated Show resolved Hide resolved
src/services/exportInfoMap.ts Outdated Show resolved Hide resolved
src/compiler/utilities.ts Outdated Show resolved Hide resolved
src/compiler/sys.ts Outdated Show resolved Hide resolved
@@ -10189,7 +10224,78 @@ export type PragmaArgumentType<KPrag extends keyof ConcretePragmaSpecs> = Concre
: never;

/** @internal */
export type ConcretePragmaSpecs = typeof commentPragmas;
export interface ConcretePragmaSpecs {
readonly "reference": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still wish we could do something simpler here, but we also don't really touch this that much. (I'm personally okay with it becoming a little less safe just to not have this duplication)

src/server/editorServices.ts Show resolved Hide resolved
@iisaduan iisaduan merged commit 52eaa7b into microsoft:main Sep 16, 2024
30 checks passed
@iisaduan iisaduan deleted the addIsolatedDecl branch September 16, 2024 21:01
@Zzzen
Copy link
Contributor

Zzzen commented Sep 23, 2024

Does isolated declarations improve the performance of the language service?

@iisaduan
Copy link
Member Author

iisaduan commented Oct 1, 2024

@Zzzen this PR shouldn't have any performance effect on our implementation of the language service. It's possible enabling it in your own code could make some parts of the LS faster in your projects, but we have not benchmarked anything or made ID specific performance improvements for the LS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Investigate Adopting --isolatedDeclarations in TypeScript's Codebase
9 participants