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

Mark file as skips typechecking if it contains ts-nocheck #58593

Merged
merged 7 commits into from
May 31, 2024

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented May 20, 2024

This is different take on #58592 to skip checking only if no-checkJs
Frankly i dont know what all will fail because we dont have enough coverage for js + incremental as such from this perspective.

cc: @weswigham

From investigating #56956

Needs #58364 to work correctly for js emit

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels May 20, 2024
@sheetalkamat sheetalkamat changed the title Skip typechecking before declaration file on js files with nocheck [Experiment] Skip typechecking before declaration file on js files with nocheck May 20, 2024
src/compiler/checker.ts Outdated Show resolved Hide resolved
@sheetalkamat sheetalkamat force-pushed the skipCheckIfNoJSCheck branch 2 times, most recently from f3189ed to 989d088 Compare May 20, 2024 22:55
@sheetalkamat sheetalkamat force-pushed the skipCheckIfNoJSCheck branch from 989d088 to ca1ff15 Compare May 20, 2024 22:57
@sheetalkamat
Copy link
Member Author

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 20, 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
Collaborator

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

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@sheetalkamat Here are the results of running the user tests comparing main and refs/pull/58593/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

@sheetalkamat
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,154 62,154 ~ ~ ~ p=1.000 n=6
Types 50,248 50,248 ~ ~ ~ p=1.000 n=6
Memory used 192,814k (± 0.76%) 192,872k (± 0.83%) ~ 192,173k 196,126k p=0.575 n=6
Parse Time 1.29s (± 1.99%) 1.29s (± 1.45%) ~ 1.27s 1.31s p=0.677 n=6
Bind Time 0.72s 0.72s ~ ~ ~ p=1.000 n=6
Check Time 9.53s (± 0.29%) 9.55s (± 0.31%) ~ 9.50s 9.59s p=0.226 n=6
Emit Time 2.65s (± 0.56%) 2.66s (± 0.50%) ~ 2.64s 2.68s p=0.285 n=6
Total Time 14.19s (± 0.27%) 14.21s (± 0.36%) ~ 14.12s 14.25s p=0.226 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,110 944,110 ~ ~ ~ p=1.000 n=6
Types 407,140 407,140 ~ ~ ~ p=1.000 n=6
Memory used 1,222,119k (± 0.00%) 1,222,101k (± 0.00%) ~ 1,222,067k 1,222,176k p=0.689 n=6
Parse Time 6.79s (± 0.68%) 6.79s (± 0.55%) ~ 6.76s 6.85s p=0.518 n=6
Bind Time 1.87s (± 0.44%) 1.88s (± 0.45%) ~ 1.86s 1.88s p=0.718 n=6
Check Time 31.34s (± 0.11%) 31.25s (± 0.74%) ~ 30.99s 31.67s p=0.128 n=6
Emit Time 14.72s (± 0.54%) 14.81s (± 0.55%) ~ 14.69s 14.92s p=0.126 n=6
Total Time 54.72s (± 0.20%) 54.73s (± 0.52%) ~ 54.39s 55.17s p=1.000 n=6
mui-docs - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 1,979,156 1,979,156 ~ ~ ~ p=1.000 n=6
Types 882,049 882,049 ~ ~ ~ p=1.000 n=6
Memory used 1,884,994k (± 0.00%) 1,884,986k (± 0.00%) ~ 1,884,947k 1,885,011k p=1.000 n=6
Parse Time 6.79s (± 0.40%) 6.78s (± 0.42%) ~ 6.75s 6.81s p=0.684 n=6
Bind Time 2.30s (± 1.37%) 2.28s (± 0.39%) ~ 2.27s 2.29s p=0.270 n=6
Check Time 60.37s (± 0.35%) 60.25s (± 0.33%) ~ 60.00s 60.59s p=0.423 n=6
Emit Time 0.14s (± 3.60%) 0.14s ~ ~ ~ p=0.174 n=6
Total Time 69.59s (± 0.32%) 69.46s (± 0.29%) ~ 69.17s 69.77s p=0.335 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,222,772 1,222,830 +58 (+ 0.00%) ~ ~ p=0.001 n=6
Types 260,022 260,027 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,338,777k (± 0.03%) 2,339,212k (± 0.03%) ~ 2,338,485k 2,340,165k p=0.173 n=6
Parse Time 5.01s (± 1.04%) 5.05s (± 1.04%) ~ 4.97s 5.11s p=0.173 n=6
Bind Time 1.89s (± 0.88%) 1.89s (± 1.23%) ~ 1.87s 1.93s p=0.933 n=6
Check Time 33.81s (± 0.35%) 33.77s (± 0.40%) ~ 33.52s 33.92s p=0.689 n=6
Emit Time 2.68s (± 2.13%) 2.68s (± 2.36%) ~ 2.57s 2.74s p=0.936 n=6
Total Time 43.40s (± 0.18%) 43.40s (± 0.44%) ~ 43.02s 43.55s p=0.378 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,222,772 1,222,830 +58 (+ 0.00%) ~ ~ p=0.001 n=6
Types 260,022 260,027 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,414,215k (± 0.05%) 2,415,044k (± 0.04%) ~ 2,413,802k 2,416,158k p=0.230 n=6
Parse Time 6.35s (± 1.12%) 6.28s (± 0.80%) ~ 6.21s 6.35s p=0.173 n=6
Bind Time 2.03s (± 0.88%) 2.04s (± 1.76%) ~ 2.00s 2.10s p=0.518 n=6
Check Time 40.22s (± 0.14%) 40.31s (± 0.31%) ~ 40.10s 40.45s p=0.109 n=6
Emit Time 3.30s (± 4.38%) 3.19s (± 2.73%) ~ 3.08s 3.27s p=0.229 n=6
Total Time 51.92s (± 0.32%) 51.82s (± 0.13%) ~ 51.71s 51.90s p=0.471 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 257,595 257,609 +14 (+ 0.01%) ~ ~ p=0.001 n=6
Types 104,862 104,867 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 426,544k (± 0.01%) 426,537k (± 0.01%) ~ 426,483k 426,620k p=0.689 n=6
Parse Time 4.17s (± 0.62%) 4.17s (± 0.25%) ~ 4.15s 4.18s p=0.357 n=6
Bind Time 1.62s (± 0.78%) 1.63s (± 0.51%) ~ 1.63s 1.65s p=0.065 n=6
Check Time 22.15s (± 0.24%) 22.20s (± 0.18%) ~ 22.13s 22.24s p=0.078 n=6
Emit Time 1.72s (± 1.89%) 1.72s (± 1.24%) ~ 1.69s 1.75s p=0.746 n=6
Total Time 29.67s (± 0.20%) 29.72s (± 0.11%) ~ 29.68s 29.77s p=0.147 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,575 224,575 ~ ~ ~ p=1.000 n=6
Types 93,785 93,785 ~ ~ ~ p=1.000 n=6
Memory used 369,858k (± 0.03%) 369,863k (± 0.02%) ~ 369,749k 369,962k p=0.936 n=6
Parse Time 2.83s (± 0.88%) 2.84s (± 1.27%) ~ 2.80s 2.88s p=0.687 n=6
Bind Time 1.58s (± 1.11%) 1.58s (± 0.48%) ~ 1.57s 1.59s p=0.397 n=6
Check Time 15.67s (± 0.47%) 15.67s (± 0.37%) ~ 15.55s 15.70s p=0.808 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.07s (± 0.45%) 20.09s (± 0.42%) ~ 19.93s 20.17s p=0.630 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,825,740 2,825,449 -291 (- 0.01%) ~ ~ p=0.001 n=6
Types 958,540 958,519 -21 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 2,999,944k (± 0.00%) 3,000,038k (± 0.00%) ~ 2,999,888k 3,000,151k p=0.093 n=6
Parse Time 13.82s (± 0.23%) 13.82s (± 0.20%) ~ 13.77s 13.84s p=1.000 n=6
Bind Time 4.14s (± 0.20%) 4.21s (± 2.83%) ~ 4.12s 4.37s p=0.867 n=6
Check Time 73.53s (± 0.28%) 73.54s (± 0.42%) ~ 73.22s 73.96s p=1.000 n=6
Emit Time 23.51s (± 0.81%) 23.64s (± 0.84%) ~ 23.40s 23.91s p=0.378 n=6
Total Time 115.01s (± 0.25%) 115.21s (± 0.14%) ~ 115.00s 115.41s p=0.149 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 265,866 265,866 ~ ~ ~ p=1.000 n=6
Types 108,401 108,401 ~ ~ ~ p=1.000 n=6
Memory used 410,663k (± 0.02%) 410,647k (± 0.02%) ~ 410,563k 410,785k p=0.872 n=6
Parse Time 4.77s (± 1.01%) 4.75s (± 0.86%) ~ 4.68s 4.79s p=0.520 n=6
Bind Time 2.07s (± 0.43%) 2.08s (± 0.95%) ~ 2.04s 2.10s p=0.198 n=6
Check Time 21.05s (± 0.35%) 20.99s (± 0.40%) ~ 20.85s 21.08s p=0.261 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 27.89s (± 0.26%) 27.82s (± 0.42%) ~ 27.61s 27.94s p=0.335 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 524,654 524,654 ~ ~ ~ p=1.000 n=6
Types 178,920 178,920 ~ ~ ~ p=1.000 n=6
Memory used 462,666k (± 0.02%) 462,740k (± 0.02%) ~ 462,647k 462,916k p=0.128 n=6
Parse Time 3.89s (± 0.25%) 3.89s (± 0.51%) ~ 3.86s 3.91s p=1.000 n=6
Bind Time 1.45s (± 1.29%) 1.44s (± 0.68%) ~ 1.43s 1.45s p=0.673 n=6
Check Time 22.48s (± 0.80%) 22.56s (± 0.50%) ~ 22.37s 22.68s p=0.422 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 27.81s (± 0.63%) 27.89s (± 0.40%) ~ 27.71s 27.99s p=0.471 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
Collaborator

@sheetalkamat Here are the results of running the top 400 repos comparing main and refs/pull/58593/merge:

Everything looks good!

@sheetalkamat sheetalkamat changed the title [Experiment] Skip typechecking before declaration file on js files with nocheck Skip typechecking for files with no check in them even with dts emit May 21, 2024
@sheetalkamat sheetalkamat marked this pull request as ready for review May 21, 2024 02:24
@sheetalkamat sheetalkamat changed the title Skip typechecking for files with no check in them even with dts emit Mark file as skips typechecking if it contains ts-nocheck May 21, 2024
@sheetalkamat
Copy link
Member Author

Does this need to wait for #58364 given the ts-nocheck files may be emitting js?

@weswigham
Copy link
Member

Does this need to wait for #58364 given the ts-nocheck files may be emitting js?

Either that or, like the other PR, only enabled when only .d.ts emit is performed by the checker in question.

@sheetalkamat
Copy link
Member Author

Either that or, like the other PR, only enabled when only .d.ts emit is performed by the checker in question.

Thats tough I think and not work flowing information temporarily till we get the PR in so i will just wait for that PR to merge.

@jakebailey
Copy link
Member

#58364 is in, so is this safe to update and get in?

@weswigham
Copy link
Member

Yeah, just remember the condition in calculateNodeCheckFlagWorker in checker.ts and emitJsFileOrBundle in emitter.ts needs to swap from just compilerOptions.noCheck to something that can handle this, too. (probably just the skipTypeChecking util function this edits, unless you wanna do the piecemeal thing emitDeclarationFileOrBundle does - but that should probably just swap to skipTypeChecking, too)

@sheetalkamat
Copy link
Member Author

sheetalkamat commented May 31, 2024

@weswigham pls review again. I have merged with main and added those checks

Also i used canIncludeBind* function instead of skipsTypeChecking to avoid having to do extra checks (or get sourceFile if not needed)

src/compiler/checker.ts Show resolved Hide resolved
src/compiler/emitter.ts Show resolved Hide resolved
src/compiler/utilities.ts Show resolved Hide resolved
@sheetalkamat
Copy link
Member Author

@weswigham pls review so i can get this in for 5.5

@sheetalkamat sheetalkamat merged commit 40583ff into main May 31, 2024
28 checks passed
@sheetalkamat sheetalkamat deleted the skipCheckIfNoJSCheck branch May 31, 2024 19:18
skeate added a commit to skeate/TypeScript that referenced this pull request Jun 1, 2024
* upstream/main: (37 commits)
  Added NoTruncation flag to completions (microsoft#58719)
  Clone node to remove location even when it has been modified if needed (microsoft#58706)
  Properly account for `this` argument in intersection apparent type caching (microsoft#58677)
  Fix: Include Values of Script Extensions for Unicode Property Value Expressions in Regular Expressions (microsoft#58615)
  In `reScanSlashToken` use `charCodeChecked` not `codePointChecked` (microsoft#58727)
  Shorten error spans for errors reported on constructor declarations (microsoft#58061)
  Mark file as skips typechecking if it contains ts-nocheck (microsoft#58593)
  Fixed an issue with broken `await using` declarations in `for of` loops (microsoft#56466)
  Do not expand type references in keyof and index access (microsoft#58715)
  Improve the performance of isolatedDeclarations quickfix  (microsoft#58722)
  Unwrap `NoInfer` types when narrowing (microsoft#58292)
  Recover from type reuse errors by falling back to inferred type printing (microsoft#58720)
  Fixing self import (microsoft#58718)
  Enable JS emit for noCheck and noCheck for transpileModule (microsoft#58364)
  Revert PR 55371 (microsoft#58702)
  Update dependencies (microsoft#58639)
  Fix baselines after PR 58621 (microsoft#58705)
  Do not infer `yield*` type from contextual `TReturn` (microsoft#58621)
  `await using` normative changes (microsoft#58624)
  Handling statements from a known source file (microsoft#58679)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants