Skip to content

preserve declarations if @internal is mentioned in unrelated comment#57960

Closed
Zzzen wants to merge 10 commits intomicrosoft:mainfrom
Zzzen:fix-internal-tag-in-comment
Closed

preserve declarations if @internal is mentioned in unrelated comment#57960
Zzzen wants to merge 10 commits intomicrosoft:mainfrom
Zzzen:fix-internal-tag-in-comment

Conversation

@Zzzen
Copy link
Copy Markdown
Contributor

@Zzzen Zzzen commented Mar 27, 2024

Fixes #57352

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Mar 27, 2024
@typescript-bot
Copy link
Copy Markdown
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.

Comment thread src/compiler/utilitiesPublic.ts Outdated
@jakebailey
Copy link
Copy Markdown
Member

Merged main for you.

As far as I'm concerned, this PR is correct; I tested dtsBundler with it and it still correctly strips internal declarations via isInternalDeclaration.

But I know we're pretty iffy on whether or not @internal is even supported or not. Only recently did I make the function to check the tag public, and this PR formalizes it into our API.

I'm personally still for it, regardless, but I'll bring this one up in a design meeting just to double check.

Comment thread src/compiler/parser.ts
Copy link
Copy Markdown
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.

I have no idea how it isn't breaking the tests, but this PR will need to modify scanner.ts to update jsDocSeeOrLink to also treat @internal as a JSDoc comment that needs to be parsed in TS files and an update to the docs for JSDocParsingMode to say that @internal is also parsed. Along with testing in jsDocParsing.ts.

Probably need to make ParseForTypeErrors the default in the compiler tests... Really thought I did that already.

Copy link
Copy Markdown
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.

Oops, meant to mark as needing changes.

@jakebailey
Copy link
Copy Markdown
Member

Close/reopening to rerun CI and show the failure.

@jakebailey
Copy link
Copy Markdown
Member

Took the liberty to apply the described change.

sheetalkamat
sheetalkamat previously approved these changes Jun 27, 2024
@jakebailey
Copy link
Copy Markdown
Member

Just checking:

@typescript-bot test it

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jun 27, 2024

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

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started 👀 Results
run dt ✅ Started 👀 Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Copy Markdown
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

There were interesting changes:

Branch only errors:

Package: react
Error:

TypeError: Cannot read properties of undefined (reading 'kind')
Occurred while linting /mnt/vss/_work/1/DefinitelyTyped/types/react/index.d.ts:7
Rule: "@definitelytyped/expect"
    at getCannotFindNameDiagnosticForName (/mnt/vss/_work/1/s/built/local/typescript.js:72148:25)
    at resolveEntityName (/mnt/vss/_work/1/s/built/local/typescript.js:52801:119)
    at resolveJSDocMemberName (/mnt/vss/_work/1/s/built/local/typescript.js:89561:20)
    at checkJSDocLinkLikeTag (/mnt/vss/_work/1/s/built/local/typescript.js:85107:7)
    at checkSourceElementWorker (/mnt/vss/_work/1/s/built/local/typescript.js:88784:16)
    at checkSourceElement (/mnt/vss/_work/1/s/built/local/typescript.js:88679:7)
    at /mnt/vss/_work/1/s/built/local/typescript.js:88888:11
    at forEach (/mnt/vss/_work/1/s/built/local/typescript.js:2402:22)
    at checkJSDocCommentWorker (/mnt/vss/_work/1/s/built/local/typescript.js:88886:7)
    at /mnt/vss/_work/1/s/built/local/typescript.js:88691:11

Package: porty
Error:

TypeError: Cannot read properties of undefined (reading 'kind')
Occurred while linting /mnt/vss/_work/1/DefinitelyTyped/types/porty/index.d.ts:1
Rule: "@definitelytyped/expect"
    at getCannotFindNameDiagnosticForName (/mnt/vss/_work/1/s/built/local/typescript.js:72148:25)
    at resolveEntityName (/mnt/vss/_work/1/s/built/local/typescript.js:52801:119)
    at resolveJSDocMemberName (/mnt/vss/_work/1/s/built/local/typescript.js:89561:20)
    at checkJSDocLinkLikeTag (/mnt/vss/_work/1/s/built/local/typescript.js:85107:7)
    at checkSourceElementWorker (/mnt/vss/_work/1/s/built/local/typescript.js:88784:16)
    at checkSourceElement (/mnt/vss/_work/1/s/built/local/typescript.js:88679:7)
    at /mnt/vss/_work/1/s/built/local/typescript.js:88888:11
    at forEach (/mnt/vss/_work/1/s/built/local/typescript.js:2402:22)
    at checkJSDocCommentWorker (/mnt/vss/_work/1/s/built/local/typescript.js:88886:7)
    at /mnt/vss/_work/1/s/built/local/typescript.js:88691:11

You can check the log here.

Copy link
Copy Markdown
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.

Those new DT errors are interesting; seems like a bug in the PR somewhere.

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/57960/merge:

Something interesting changed - please have a look.

Details

azure-sdk

/mnt/ts_downloads/_/m/azure-sdk/build.sh

  • [MISSING] error TS2322: Type 'string' is not assignable to type 'never'.
    • /mnt/ts_downloads/_/m/azure-sdk/test/narrowedTypes.ts(59,9)
    • /mnt/ts_downloads/_/m/azure-sdk/test/narrowedTypes.ts(213,9)
    • /mnt/ts_downloads/_/m/azure-sdk/test/narrowedTypes.ts(254,11)

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey
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,243 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 193,456k (± 0.91%) 194,017k (± 0.01%) ~ 193,992k 194,044k p=0.378 n=6
Parse Time 1.32s (± 0.57%) 1.28s (± 0.85%) -0.04s (- 2.91%) 1.26s 1.29s p=0.004 n=6
Bind Time 0.71s 0.73s +0.02s (+ 2.82%) ~ ~ p=0.001 n=6
Check Time 9.42s (± 0.21%) 9.45s (± 0.45%) ~ 9.39s 9.50s p=0.296 n=6
Emit Time 2.76s (± 0.44%) 2.96s (± 0.72%) 🔻+0.20s (+ 7.38%) 2.93s 2.98s p=0.004 n=6
Total Time 14.21s (± 0.12%) 14.41s (± 0.35%) +0.21s (+ 1.45%) 14.32s 14.46s p=0.005 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,114 944,114 ~ ~ ~ p=1.000 n=6
Types 407,050 407,050 ~ ~ ~ p=1.000 n=6
Memory used 1,218,315k (± 0.00%) 1,218,668k (± 0.00%) +353k (+ 0.03%) 1,218,623k 1,218,745k p=0.005 n=6
Parse Time 6.68s (± 0.43%) 6.72s (± 0.51%) ~ 6.67s 6.77s p=0.109 n=6
Bind Time 1.86s (± 0.65%) 1.88s (± 0.43%) +0.02s (+ 1.07%) 1.87s 1.89s p=0.017 n=6
Check Time 30.61s (± 0.44%) 30.62s (± 0.42%) ~ 30.43s 30.79s p=1.000 n=6
Emit Time 13.57s (± 0.29%) 13.55s (± 0.55%) ~ 13.44s 13.67s p=0.568 n=6
Total Time 52.72s (± 0.27%) 52.77s (± 0.28%) ~ 52.58s 52.93s p=0.575 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,133,050 2,133,050 ~ ~ ~ p=1.000 n=6
Types 926,444 926,444 ~ ~ ~ p=1.000 n=6
Memory used 2,115,475k (± 0.01%) 2,115,640k (± 0.01%) +166k (+ 0.01%) 2,115,488k 2,115,734k p=0.045 n=6
Parse Time 6.63s (± 0.60%) 6.63s (± 0.48%) ~ 6.57s 6.67s p=0.936 n=6
Bind Time 2.31s (± 1.31%) 2.31s (± 1.11%) ~ 2.28s 2.34s p=0.934 n=6
Check Time 70.75s (± 0.26%) 70.92s (± 0.61%) ~ 70.44s 71.56s p=0.689 n=6
Emit Time 0.13s (± 3.87%) 0.14s (± 4.51%) ~ 0.13s 0.15s p=0.091 n=6
Total Time 79.83s (± 0.21%) 79.99s (± 0.50%) ~ 79.50s 80.55s p=0.630 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,231,640 1,231,878 +238 (+ 0.02%) ~ ~ p=0.001 n=6
Types 261,187 261,206 +19 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,347,778k (± 0.02%) 2,385,972k (± 0.04%) +38,193k (+ 1.63%) 2,384,749k 2,386,783k p=0.005 n=6
Parse Time 5.05s (± 1.08%) 5.37s (± 0.66%) 🔻+0.33s (+ 6.47%) 5.32s 5.41s p=0.005 n=6
Bind Time 1.92s (± 1.29%) 1.95s (± 0.65%) +0.03s (+ 1.65%) 1.93s 1.96s p=0.024 n=6
Check Time 34.28s (± 0.20%) 34.18s (± 0.21%) ~ 34.09s 34.25s p=0.093 n=6
Emit Time 2.72s (± 2.15%) 2.73s (± 2.77%) ~ 2.66s 2.85s p=0.936 n=6
Total Time 43.96s (± 0.12%) 44.24s (± 0.19%) +0.28s (+ 0.63%) 44.14s 44.39s 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,231,640 1,231,878 +238 (+ 0.02%) ~ ~ p=0.001 n=6
Types 261,187 261,206 +19 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,423,712k (± 0.07%) 2,462,611k (± 0.04%) +38,899k (+ 1.60%) 2,460,874k 2,463,556k p=0.005 n=6
Parse Time 6.25s (± 0.88%) 6.59s (± 0.73%) 🔻+0.33s (+ 5.33%) 6.54s 6.67s p=0.005 n=6
Bind Time 2.05s (± 0.27%) 2.32s (± 1.48%) 🔻+0.28s (+13.53%) 2.27s 2.36s p=0.005 n=6
Check Time 40.54s (± 0.33%) 40.26s (± 0.24%) -0.28s (- 0.70%) 40.11s 40.38s p=0.005 n=6
Emit Time 3.29s (± 4.48%) 3.17s (± 1.60%) ~ 3.13s 3.27s p=0.065 n=6
Total Time 52.14s (± 0.42%) 52.36s (± 0.21%) ~ 52.26s 52.54s p=0.065 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,818 258,836 +18 (+ 0.01%) ~ ~ p=0.001 n=6
Types 104,842 104,853 +11 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 428,245k (± 0.00%) 430,708k (± 0.01%) +2,463k (+ 0.58%) 430,665k 430,783k p=0.005 n=6
Parse Time 4.09s (± 0.45%) 4.26s (± 0.69%) +0.16s (+ 3.99%) 4.22s 4.29s p=0.005 n=6
Bind Time 1.60s (± 1.07%) 1.63s (± 0.98%) +0.03s (+ 1.87%) 1.62s 1.66s p=0.023 n=6
Check Time 22.00s (± 0.46%) 21.97s (± 0.31%) ~ 21.88s 22.06s p=0.689 n=6
Emit Time 1.72s (± 1.67%) 1.69s (± 1.32%) ~ 1.67s 1.72s p=0.138 n=6
Total Time 29.42s (± 0.32%) 29.55s (± 0.19%) +0.13s (+ 0.46%) 29.49s 29.62s p=0.030 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,734 ~ ~ ~ p=1.000 n=6
Memory used 369,444k (± 0.02%) 369,507k (± 0.01%) ~ 369,433k 369,586k p=0.173 n=6
Parse Time 2.77s (± 1.36%) 2.71s (± 0.63%) -0.05s (- 1.99%) 2.68s 2.73s p=0.012 n=6
Bind Time 1.58s (± 0.93%) 1.58s (± 0.74%) ~ 1.57s 1.60s p=0.933 n=6
Check Time 15.51s (± 0.34%) 15.48s (± 0.24%) ~ 15.43s 15.52s p=0.257 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 19.86s (± 0.32%) 19.77s (± 0.23%) -0.09s (- 0.46%) 19.71s 19.81s p=0.036 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,878,629 2,878,629 ~ ~ ~ p=1.000 n=6
Types 975,177 975,177 ~ ~ ~ p=1.000 n=6
Memory used 3,042,150k (± 0.00%) 3,042,429k (± 0.00%) +279k (+ 0.01%) 3,042,394k 3,042,459k p=0.005 n=6
Parse Time 13.57s (± 0.44%) 13.60s (± 0.35%) ~ 13.56s 13.66s p=0.226 n=6
Bind Time 4.22s (± 1.83%) 4.22s (± 2.03%) ~ 4.16s 4.39s p=1.000 n=6
Check Time 73.36s (± 0.43%) 73.60s (± 1.74%) ~ 72.72s 76.18s p=0.468 n=6
Emit Time 24.08s (± 0.47%) 23.51s (± 4.72%) -0.57s (- 2.39%) 21.25s 24.06s p=0.037 n=6
Total Time 115.22s (± 0.29%) 114.93s (± 0.28%) ~ 114.46s 115.42s p=0.230 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,117 267,117 ~ ~ ~ p=1.000 n=6
Types 108,775 108,775 ~ ~ ~ p=1.000 n=6
Memory used 411,547k (± 0.01%) 411,614k (± 0.02%) ~ 411,539k 411,739k p=0.109 n=6
Parse Time 3.84s (± 0.63%) 3.84s (± 0.58%) ~ 3.81s 3.87s p=0.871 n=6
Bind Time 1.70s (± 0.81%) 1.69s (± 0.65%) ~ 1.68s 1.71s p=0.491 n=6
Check Time 16.76s (± 0.36%) 16.75s (± 0.24%) ~ 16.69s 16.80s p=0.419 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.30s (± 0.36%) 22.29s (± 0.11%) ~ 22.24s 22.30s p=0.361 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 525,251 525,251 ~ ~ ~ p=1.000 n=6
Types 178,574 178,574 ~ ~ ~ p=1.000 n=6
Memory used 462,735k (± 0.08%) 462,930k (± 0.07%) ~ 462,418k 463,282k p=0.378 n=6
Parse Time 3.17s (± 0.63%) 3.16s (± 1.29%) ~ 3.11s 3.22s p=0.628 n=6
Bind Time 1.16s (± 1.04%) 1.17s (± 0.54%) ~ 1.16s 1.18s p=0.340 n=6
Check Time 17.94s (± 0.42%) 17.90s (± 0.34%) ~ 17.82s 17.97s p=0.378 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.27s (± 0.32%) 22.23s (± 0.42%) ~ 22.11s 22.36s p=0.466 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

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/57960/merge:

Everything looks good!

@Zzzen Zzzen closed this Jun 28, 2024
@Zzzen Zzzen reopened this Jun 28, 2024
@Zzzen
Copy link
Copy Markdown
Contributor Author

Zzzen commented Jun 28, 2024

The DT errors should have been fixed.

@jakebailey
Copy link
Copy Markdown
Member

@typescript-bot test it

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jun 28, 2024

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

Command Status Results
test top400 ✅ Started
user test this ✅ Started
run dt ✅ Started
perf test this faster ✅ Started

@jakebailey
Copy link
Copy Markdown
Member

@typescript-bot test it

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jun 28, 2024

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

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Copy Markdown
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/57960/merge:

Everything looks good!

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey
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,243 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 192,920k (± 0.73%) 194,749k (± 0.75%) +1,829k (+ 0.95%) 193,998k 197,702k p=0.045 n=6
Parse Time 1.31s (± 0.84%) 1.29s (± 0.63%) -0.02s (- 1.78%) 1.28s 1.30s p=0.011 n=6
Bind Time 0.71s 0.73s (± 0.56%) +0.02s (+ 2.58%) 0.72s 0.73s p=0.002 n=6
Check Time 9.44s (± 0.32%) 9.44s (± 0.32%) ~ 9.40s 9.47s p=1.000 n=6
Emit Time 2.74s (± 0.38%) 2.96s (± 0.56%) 🔻+0.22s (+ 8.04%) 2.93s 2.97s p=0.005 n=6
Total Time 14.19s (± 0.25%) 14.41s (± 0.13%) +0.21s (+ 1.51%) 14.39s 14.44s p=0.005 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,114 944,114 ~ ~ ~ p=1.000 n=6
Types 407,050 407,050 ~ ~ ~ p=1.000 n=6
Memory used 1,218,352k (± 0.00%) 1,218,687k (± 0.00%) +336k (+ 0.03%) 1,218,631k 1,218,732k p=0.005 n=6
Parse Time 7.97s (± 0.65%) 8.02s (± 0.45%) +0.05s (+ 0.63%) 7.98s 8.08s p=0.045 n=6
Bind Time 2.22s (± 0.53%) 2.25s (± 0.33%) +0.03s (+ 1.35%) 2.24s 2.26s p=0.007 n=6
Check Time 35.80s (± 0.26%) 35.88s (± 0.28%) ~ 35.75s 36.03s p=0.225 n=6
Emit Time 16.14s (± 0.32%) 16.21s (± 0.45%) ~ 16.08s 16.29s p=0.077 n=6
Total Time 62.12s (± 0.16%) 62.35s (± 0.17%) +0.22s (+ 0.36%) 62.20s 62.47s p=0.016 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,133,039 2,133,039 ~ ~ ~ p=1.000 n=6
Types 926,444 926,444 ~ ~ ~ p=1.000 n=6
Memory used 2,115,329k (± 0.00%) 2,115,454k (± 0.00%) +126k (+ 0.01%) 2,115,385k 2,115,525k p=0.005 n=6
Parse Time 7.92s (± 0.26%) 7.91s (± 0.53%) ~ 7.86s 7.97s p=0.466 n=6
Bind Time 2.76s (± 0.42%) 2.76s (± 0.32%) ~ 2.75s 2.77s p=0.933 n=6
Check Time 83.41s (± 0.55%) 83.31s (± 0.68%) ~ 82.64s 83.98s p=0.810 n=6
Emit Time 0.16s (± 4.75%) 0.15s (± 5.39%) ~ 0.15s 0.17s p=0.432 n=6
Total Time 94.25s (± 0.49%) 94.13s (± 0.60%) ~ 93.44s 94.77s p=0.748 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,224,331 1,224,580 +249 (+ 0.02%) ~ ~ p=0.001 n=6
Types 261,007 261,032 +25 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,337,769k (± 0.05%) 2,374,737k (± 0.02%) +36,968k (+ 1.58%) 2,374,000k 2,375,421k p=0.005 n=6
Parse Time 5.06s (± 0.90%) 5.35s (± 0.62%) 🔻+0.28s (+ 5.56%) 5.30s 5.40s p=0.005 n=6
Bind Time 1.90s (± 1.13%) 1.94s (± 1.01%) +0.03s (+ 1.75%) 1.91s 1.97s p=0.023 n=6
Check Time 34.16s (± 0.66%) 34.18s (± 0.20%) ~ 34.09s 34.25s p=0.230 n=6
Emit Time 2.73s (± 3.11%) 2.72s (± 2.83%) ~ 2.65s 2.83s p=0.810 n=6
Total Time 43.88s (± 0.65%) 44.21s (± 0.31%) ~ 44.09s 44.39s p=0.066 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,224,331 1,224,580 +249 (+ 0.02%) ~ ~ p=0.001 n=6
Types 261,007 261,032 +25 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,412,649k (± 0.05%) 2,450,417k (± 0.04%) +37,768k (+ 1.57%) 2,449,039k 2,451,641k p=0.005 n=6
Parse Time 6.28s (± 0.88%) 6.60s (± 1.02%) 🔻+0.32s (+ 5.15%) 6.52s 6.68s p=0.005 n=6
Bind Time 2.02s (± 0.86%) 2.21s (± 6.02%) 🔻+0.19s (+ 9.31%) 2.04s 2.31s p=0.007 n=6
Check Time 40.74s (± 0.17%) 40.46s (± 0.40%) -0.28s (- 0.69%) 40.28s 40.68s p=0.008 n=6
Emit Time 3.28s (± 2.53%) 3.24s (± 1.60%) ~ 3.16s 3.29s p=0.471 n=6
Total Time 52.34s (± 0.32%) 52.50s (± 0.14%) +0.17s (+ 0.32%) 52.41s 52.62s p=0.045 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,166 258,195 +29 (+ 0.01%) ~ ~ p=0.001 n=6
Types 104,689 104,706 +17 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 427,333k (± 0.00%) 429,739k (± 0.01%) +2,406k (+ 0.56%) 429,670k 429,782k p=0.005 n=6
Parse Time 4.06s (± 0.21%) 4.28s (± 0.45%) 🔻+0.21s (+ 5.25%) 4.25s 4.30s p=0.004 n=6
Bind Time 1.63s (± 1.38%) 1.64s (± 0.71%) ~ 1.63s 1.66s p=0.324 n=6
Check Time 21.96s (± 0.36%) 21.91s (± 0.14%) ~ 21.86s 21.93s p=0.070 n=6
Emit Time 1.75s (± 1.20%) 1.75s (± 1.19%) ~ 1.72s 1.78s p=0.742 n=6
Total Time 29.40s (± 0.33%) 29.57s (± 0.06%) +0.17s (+ 0.59%) 29.55s 29.60s p=0.045 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,734 ~ ~ ~ p=1.000 n=6
Memory used 369,551k (± 0.02%) 369,607k (± 0.02%) ~ 369,518k 369,668k p=0.093 n=6
Parse Time 2.30s (± 0.81%) 2.25s (± 0.46%) -0.04s (- 1.81%) 2.24s 2.27s p=0.006 n=6
Bind Time 1.33s (± 0.88%) 1.30s (± 0.64%) -0.02s (- 1.76%) 1.30s 1.32s p=0.008 n=6
Check Time 13.15s (± 0.30%) 13.16s (± 0.37%) ~ 13.08s 13.21s p=0.629 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 16.78s (± 0.17%) 16.72s (± 0.28%) ~ 16.66s 16.79s p=0.065 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,878,940 2,878,940 ~ ~ ~ p=1.000 n=6
Types 975,074 975,074 ~ ~ ~ p=1.000 n=6
Memory used 3,041,723k (± 0.00%) 3,042,020k (± 0.00%) +297k (+ 0.01%) 3,041,878k 3,042,123k p=0.005 n=6
Parse Time 11.40s (± 0.26%) 11.49s (± 0.39%) +0.10s (+ 0.85%) 11.45s 11.55s p=0.005 n=6
Bind Time 3.64s (± 2.46%) 3.57s (± 2.28%) ~ 3.51s 3.69s p=0.189 n=6
Check Time 63.80s (± 1.27%) 63.34s (± 0.36%) ~ 63.04s 63.74s p=0.470 n=6
Emit Time 20.21s (± 3.48%) 20.65s (± 0.78%) ~ 20.35s 20.79s p=0.173 n=6
Total Time 99.04s (± 0.13%) 99.06s (± 0.36%) ~ 98.80s 99.76s p=0.378 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,117 267,117 ~ ~ ~ p=1.000 n=6
Types 108,775 108,775 ~ ~ ~ p=1.000 n=6
Memory used 411,592k (± 0.01%) 411,639k (± 0.03%) ~ 411,505k 411,815k p=1.000 n=6
Parse Time 3.81s (± 0.52%) 3.82s (± 0.77%) ~ 3.77s 3.85s p=0.418 n=6
Bind Time 1.68s (± 0.62%) 1.69s (± 0.48%) ~ 1.68s 1.70s p=0.865 n=6
Check Time 16.70s (± 0.41%) 16.75s (± 0.36%) ~ 16.68s 16.85s p=0.419 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.20s (± 0.35%) 22.26s (± 0.30%) ~ 22.19s 22.37s p=0.173 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 525,251 525,251 ~ ~ ~ p=1.000 n=6
Types 178,574 178,574 ~ ~ ~ p=1.000 n=6
Memory used 462,924k (± 0.07%) 462,818k (± 0.07%) ~ 462,389k 463,191k p=0.575 n=6
Parse Time 3.19s (± 0.59%) 3.19s (± 0.47%) ~ 3.16s 3.20s p=0.935 n=6
Bind Time 1.17s (± 0.44%) 1.17s (± 0.94%) ~ 1.16s 1.19s p=0.324 n=6
Check Time 17.95s (± 0.35%) 17.93s (± 0.24%) ~ 17.87s 17.98s p=0.748 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.31s (± 0.30%) 22.29s (± 0.22%) ~ 22.22s 22.35s p=0.809 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

@jakebailey
Copy link
Copy Markdown
Member

Perf is unfortunately still greatly affected and needs to be looked into.

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/57960/merge:

Everything looks good!

@Zzzen
Copy link
Copy Markdown
Contributor Author

Zzzen commented Jun 29, 2024

Perf problem is likely caused by the regex change.

const jsDocNeededInTypeScriptFile = /@(?:see|link|internal)/i; is 6% slower than the original one.

perf.link

@jakebailey
Copy link
Copy Markdown
Member

Funnily I clicked that link and reran it, and it told me that the new one was 6% faster; I think this particular result is just noisy. Given the benchmark is consistent with so few samples, a profile may provide more insight (which I plan to test).

@Zzzen
Copy link
Copy Markdown
Contributor Author

Zzzen commented Jun 29, 2024

LOL. After I executed it many times, I also found that the output result is somewhat random.

@sandersn
Copy link
Copy Markdown
Member

sandersn commented Jul 9, 2024

Do the random perf results mean this is safe to merge? I'd guess not, unless it's a problem with the perf infrastructure.

@jakebailey
Copy link
Copy Markdown
Member

No, there's nothing wrong with the perf infra, there really is a slowdown here that needs to be profiled.

@jakebailey
Copy link
Copy Markdown
Member

@typescript-bot perf test this faster

I can't seem to get the slowdown locally...

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jul 9, 2024

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

Command Status Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Copy Markdown
Collaborator

@jakebailey
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,243 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 193,869k (± 1.04%) 194,705k (± 0.79%) ~ 193,996k 197,837k p=0.689 n=6
Parse Time 1.96s (± 0.56%) 1.93s (± 1.02%) -0.03s (- 1.28%) 1.92s 1.96s p=0.036 n=6
Bind Time 1.06s (± 0.84%) 1.08s (± 0.70%) +0.02s (+ 1.73%) 1.07s 1.09s p=0.011 n=6
Check Time 13.85s (± 0.50%) 13.87s (± 0.30%) ~ 13.83s 13.94s p=0.419 n=6
Emit Time 4.07s (± 3.39%) 4.36s (± 1.42%) 🔻+0.30s (+ 7.29%) 4.30s 4.47s p=0.016 n=6
Total Time 20.94s (± 0.74%) 21.25s (± 0.34%) +0.31s (+ 1.49%) 21.17s 21.34s p=0.020 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,114 944,114 ~ ~ ~ p=1.000 n=6
Types 407,050 407,050 ~ ~ ~ p=1.000 n=6
Memory used 1,218,365k (± 0.00%) 1,218,704k (± 0.00%) +338k (+ 0.03%) 1,218,639k 1,218,746k p=0.005 n=6
Parse Time 7.97s (± 0.39%) 8.00s (± 0.39%) ~ 7.95s 8.03s p=0.126 n=6
Bind Time 2.21s (± 0.25%) 2.22s (± 0.66%) ~ 2.20s 2.24s p=0.091 n=6
Check Time 35.66s (± 0.16%) 35.70s (± 0.18%) ~ 35.60s 35.79s p=0.378 n=6
Emit Time 16.08s (± 0.96%) 16.13s (± 0.27%) ~ 16.06s 16.18s p=0.128 n=6
Total Time 61.92s (± 0.24%) 62.04s (± 0.13%) ~ 61.94s 62.13s p=0.066 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,130,342 2,130,342 ~ ~ ~ p=1.000 n=6
Types 927,684 927,684 ~ ~ ~ p=1.000 n=6
Memory used 2,107,288k (± 0.01%) 2,107,326k (± 0.01%) ~ 2,107,056k 2,107,557k p=0.689 n=6
Parse Time 9.71s (± 0.37%) 9.64s (± 0.85%) ~ 9.49s 9.72s p=0.078 n=6
Bind Time 3.41s (± 0.66%) 3.38s (± 0.77%) ~ 3.34s 3.41s p=0.145 n=6
Check Time 101.20s (± 1.39%) 102.15s (± 1.51%) ~ 99.07s 103.10s p=0.298 n=6
Emit Time 0.19s (± 4.22%) 0.40s (±126.52%) ~ 0.19s 1.44s p=0.340 n=6
Total Time 114.50s (± 1.22%) 115.57s (± 0.91%) ~ 113.53s 116.43s p=0.173 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,784 1,226,033 +249 (+ 0.02%) ~ ~ p=0.001 n=6
Types 261,518 261,543 +25 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,341,759k (± 0.03%) 2,378,991k (± 0.04%) +37,233k (+ 1.59%) 2,377,759k 2,380,394k p=0.005 n=6
Parse Time 6.03s (± 0.72%) 6.39s (± 0.48%) 🔻+0.36s (+ 5.91%) 6.33s 6.42s p=0.005 n=6
Bind Time 2.26s (± 0.87%) 2.29s (± 0.22%) +0.03s (+ 1.33%) 2.29s 2.30s p=0.012 n=6
Check Time 40.27s (± 0.73%) 40.30s (± 0.67%) ~ 39.88s 40.70s p=0.471 n=6
Emit Time 3.08s (± 2.37%) 3.25s (± 5.44%) ~ 3.04s 3.48s p=0.054 n=6
Total Time 51.66s (± 0.71%) 52.24s (± 0.47%) +0.59s (+ 1.14%) 51.97s 52.58s p=0.031 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,784 1,226,033 +249 (+ 0.02%) ~ ~ p=0.001 n=6
Types 261,518 261,543 +25 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,414,879k (± 0.04%) 2,451,359k (± 0.07%) +36,480k (+ 1.51%) 2,449,510k 2,453,559k p=0.005 n=6
Parse Time 6.28s (± 1.01%) 6.61s (± 0.87%) 🔻+0.32s (+ 5.14%) 6.54s 6.69s p=0.005 n=6
Bind Time 2.02s (± 0.96%) 2.27s (± 3.50%) 🔻+0.25s (+12.30%) 2.11s 2.32s p=0.005 n=6
Check Time 40.65s (± 0.35%) 40.29s (± 0.53%) -0.36s (- 0.87%) 39.99s 40.58s p=0.013 n=6
Emit Time 3.29s (± 4.80%) 3.31s (± 4.85%) ~ 3.12s 3.57s p=0.810 n=6
Total Time 52.24s (± 0.51%) 52.49s (± 0.61%) ~ 51.96s 52.88s p=0.128 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,268 258,297 +29 (+ 0.01%) ~ ~ p=0.001 n=6
Types 104,775 104,792 +17 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 427,517k (± 0.02%) 429,920k (± 0.01%) +2,403k (+ 0.56%) 429,874k 429,950k p=0.005 n=6
Parse Time 4.09s (± 0.70%) 4.27s (± 0.34%) 🔻+0.18s (+ 4.40%) 4.25s 4.29s p=0.005 n=6
Bind Time 1.61s (± 1.28%) 1.66s (± 0.83%) +0.04s (+ 2.48%) 1.63s 1.67s p=0.012 n=6
Check Time 22.05s (± 0.27%) 22.00s (± 0.34%) ~ 21.92s 22.12s p=0.334 n=6
Emit Time 1.58s (± 1.30%) 1.57s (± 0.94%) ~ 1.55s 1.59s p=0.289 n=6
Total Time 29.34s (± 0.34%) 29.50s (± 0.27%) +0.17s (+ 0.57%) 29.40s 29.62s p=0.013 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,734 ~ ~ ~ p=1.000 n=6
Memory used 369,515k (± 0.02%) 369,637k (± 0.03%) ~ 369,518k 369,825k p=0.173 n=6
Parse Time 3.44s (± 0.88%) 3.40s (± 1.06%) ~ 3.36s 3.45s p=0.148 n=6
Bind Time 1.94s (± 1.25%) 1.92s (± 0.97%) ~ 1.90s 1.95s p=0.142 n=6
Check Time 19.14s (± 0.22%) 19.18s (± 0.25%) ~ 19.12s 19.25s p=0.261 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.52s (± 0.20%) 24.50s (± 0.21%) ~ 24.46s 24.59s p=0.570 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,887,752 2,887,752 ~ ~ ~ p=1.000 n=6
Types 977,127 977,127 ~ ~ ~ p=1.000 n=6
Memory used 3,051,720k (± 0.00%) 3,052,001k (± 0.00%) +281k (+ 0.01%) 3,051,944k 3,052,066k p=0.005 n=6
Parse Time 13.75s (± 0.31%) 13.78s (± 0.22%) ~ 13.75s 13.83s p=0.373 n=6
Bind Time 4.19s (± 0.23%) 4.20s (± 0.18%) +0.01s (+ 0.32%) 4.19s 4.21s p=0.044 n=6
Check Time 74.52s (± 2.19%) 73.95s (± 0.33%) ~ 73.61s 74.23s p=1.000 n=6
Emit Time 23.28s (± 7.09%) 23.91s (± 0.33%) ~ 23.77s 24.01s p=0.873 n=6
Total Time 115.75s (± 0.35%) 115.84s (± 0.23%) ~ 115.53s 116.18s p=0.689 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,117 267,117 ~ ~ ~ p=1.000 n=6
Types 108,775 108,775 ~ ~ ~ p=1.000 n=6
Memory used 411,541k (± 0.02%) 411,644k (± 0.02%) ~ 411,550k 411,715k p=0.066 n=6
Parse Time 4.72s (± 0.68%) 4.72s (± 0.72%) ~ 4.68s 4.77s p=1.000 n=6
Bind Time 2.09s (± 0.64%) 2.08s (± 0.84%) ~ 2.07s 2.11s p=0.510 n=6
Check Time 20.76s (± 0.30%) 20.82s (± 0.28%) ~ 20.72s 20.90s p=0.125 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 27.57s (± 0.20%) 27.62s (± 0.34%) ~ 27.48s 27.76s p=0.227 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 525,251 525,251 ~ ~ ~ p=1.000 n=6
Types 178,574 178,574 ~ ~ ~ p=1.000 n=6
Memory used 462,531k (± 0.06%) 462,903k (± 0.06%) ~ 462,448k 463,133k p=0.065 n=6
Parse Time 2.65s (± 0.60%) 2.64s (± 0.62%) ~ 2.62s 2.66s p=0.190 n=6
Bind Time 0.98s (± 0.53%) 0.97s (± 0.84%) -0.01s (- 1.02%) 0.96s 0.98s p=0.050 n=6
Check Time 15.16s (± 0.33%) 15.22s (± 0.23%) +0.06s (+ 0.41%) 15.18s 15.28s p=0.037 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 18.79s (± 0.24%) 18.82s (± 0.21%) ~ 18.76s 18.86s p=0.170 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

@github-project-automation github-project-automation Bot moved this from Waiting on reviewers to Done in PR Backlog Mar 24, 2026
@typescript-bot
Copy link
Copy Markdown
Collaborator

With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies).

Next steps for PRs:

  • For crash bugfixes or language service improvements, PRs are currently accepted at the typescript-go repo
  • Changes to type system behavior should wait until after 7.0, at which point mainline TypeScript development will resume in this repository with the Go codebase
  • Library file updates (lib.d.ts etc) continue to live in this repo or the DOM Generator repo as appropriate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Backlog Bug PRs that fix a backlog bug

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Class member incorrectly stripped from .d.ts output if @internal is mentioned in unrelated comment

6 participants