Skip to content

fix(55994): Type-check Import Attributes in static imports #56034

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 10, 2024
Merged

fix(55994): Type-check Import Attributes in static imports #56034

merged 4 commits into from
Jan 10, 2024

Conversation

a-tarasyuk
Copy link
Contributor

Fixes #55994

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Oct 8, 2023
@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.

jakebailey
jakebailey reviewed Oct 9, 2023
View reviewed changes
@a-tarasyuk a-tarasyuk changed the title [WIP] fix(55994): Type-check Import Attributes in static imports fix(55994): Type-check Import Attributes in static imports Jan 7, 2024
@a-tarasyuk a-tarasyuk marked this pull request as ready for review January 7, 2024 22:59
@a-tarasyuk a-tarasyuk requested a review from jakebailey January 7, 2024 22:59
@jakebailey
Copy link
Member

LGTM, but I would like a second opinion.

@typescript-bot test top200
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot perf test this
@typescript-bot pack this

@typescript-bot user test tsserver
@typescript-bot test tsserver top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the tarball bundle task on this PR at baf8ac2. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the diff-based top-repos suite (tsserver) on this PR at baf8ac2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the regular perf test suite on this PR at baf8ac2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at baf8ac2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at baf8ac2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the diff-based user code test suite (tsserver) on this PR at baf8ac2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at baf8ac2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 8, 2024 •
edited
Loading

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/159329/artifacts?artifactName=tgz&fileId=5B001DC2A6D1EF32082EFC28AD2BCEDD000F430F3A95CE9050B55729D6FC0DFF02&fileName=/typescript-5.4.0-insiders.20240108.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/56034/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/56034/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot
Copy link
Collaborator

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

Here they are:

startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v18.15.0, x64)
Execution time 153.74ms (± 0.20%) 153.70ms (± 0.20%) ~ 152.36ms 158.16ms p=0.361 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 229.02ms (± 0.16%) 229.10ms (± 0.19%) ~ 227.52ms 237.20ms p=0.215 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 231.97ms (± 0.19%) 232.17ms (± 0.20%) +0.19ms (+ 0.08%) 230.16ms 240.35ms p=0.000 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 230.68ms (± 0.18%) 230.64ms (± 0.18%) ~ 229.14ms 234.91ms p=0.226 n=600
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • tsc-startup - node (v18.15.0, x64)
  • tsserver-startup - node (v18.15.0, x64)
  • tsserverlibrary-startup - node (v18.15.0, x64)
  • typescript-startup - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v18.15.0, x64)
Memory used 295,487k (± 0.01%) 295,458k (± 0.01%) ~ 295,421k 295,501k p=0.093 n=6
Parse Time 2.65s (± 0.21%) 2.65s (± 0.31%) ~ 2.64s 2.66s p=0.859 n=6
Bind Time 0.82s (± 0.50%) 0.83s (± 1.25%) ~ 0.82s 0.84s p=0.114 n=6
Check Time 8.16s (± 0.32%) 8.14s (± 0.38%) ~ 8.09s 8.17s p=0.146 n=6
Emit Time 7.11s (± 0.33%) 7.10s (± 0.24%) ~ 7.07s 7.12s p=0.368 n=6
Total Time 18.74s (± 0.19%) 18.71s (± 0.21%) ~ 18.66s 18.75s p=0.196 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 192,490k (± 1.23%) 194,545k (± 1.70%) ~ 191,473k 197,620k p=0.230 n=6
Parse Time 1.36s (± 1.43%) 1.35s (± 1.30%) ~ 1.33s 1.38s p=0.737 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.24s (± 0.19%) 9.27s (± 0.11%) +0.03s (+ 0.34%) 9.26s 9.28s p=0.011 n=6
Emit Time 2.63s (± 0.74%) 2.63s (± 0.85%) ~ 2.60s 2.65s p=0.871 n=6
Total Time 13.95s (± 0.07%) 13.97s (± 0.27%) ~ 13.93s 14.02s p=0.373 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,401k (± 0.01%) 347,421k (± 0.00%) +21k (+ 0.01%) 347,402k 347,435k p=0.045 n=6
Parse Time 2.46s (± 0.49%) 2.46s (± 0.48%) ~ 2.44s 2.47s p=0.868 n=6
Bind Time 0.93s (± 0.00%) 0.93s (± 0.44%) ~ 0.92s 0.93s p=0.405 n=6
Check Time 6.86s (± 0.29%) 6.88s (± 0.57%) ~ 6.84s 6.95s p=0.745 n=6
Emit Time 4.05s (± 0.30%) 4.05s (± 0.40%) ~ 4.03s 4.07s p=0.365 n=6
Total Time 14.30s (± 0.13%) 14.31s (± 0.34%) ~ 14.26s 14.40s p=0.746 n=6
TFS - node (v18.15.0, x64)
Memory used 302,745k (± 0.00%) 302,734k (± 0.01%) ~ 302,715k 302,771k p=0.261 n=6
Parse Time 1.99s (± 0.97%) 2.00s (± 0.86%) ~ 1.98s 2.02s p=0.325 n=6
Bind Time 1.00s (± 0.00%) 1.00s (± 0.81%) ~ 0.99s 1.01s p=0.290 n=6
Check Time 6.30s (± 0.52%) 6.29s (± 0.30%) ~ 6.26s 6.31s p=0.871 n=6
Emit Time 3.57s (± 0.27%) 3.58s (± 0.35%) ~ 3.56s 3.59s p=0.270 n=6
Total Time 12.86s (± 0.32%) 12.88s (± 0.31%) ~ 12.81s 12.92s p=0.258 n=6
material-ui - node (v18.15.0, x64)
Memory used 506,812k (± 0.00%) 506,819k (± 0.01%) ~ 506,781k 506,863k p=0.575 n=6
Parse Time 2.59s (± 0.80%) 2.59s (± 0.40%) ~ 2.57s 2.60s p=0.621 n=6
Bind Time 0.99s (± 1.61%) 0.99s (± 0.76%) ~ 0.98s 1.00s p=1.000 n=6
Check Time 16.96s (± 0.28%) 16.91s (± 0.46%) ~ 16.81s 17.02s p=0.296 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.54s (± 0.24%) 20.49s (± 0.38%) ~ 20.40s 20.59s p=0.336 n=6
xstate - node (v18.15.0, x64)
Memory used 512,837k (± 0.01%) 512,861k (± 0.01%) ~ 512,785k 512,943k p=0.689 n=6
Parse Time 3.27s (± 0.26%) 3.28s (± 0.12%) ~ 3.27s 3.28s p=0.285 n=6
Bind Time 1.54s (± 0.53%) 1.54s (± 0.34%) ~ 1.53s 1.54s p=0.929 n=6
Check Time 2.84s (± 0.85%) 2.83s (± 0.47%) ~ 2.82s 2.85s p=0.415 n=6
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) ~ 0.07s 0.07s p=1.000 n=6
Total Time 7.72s (± 0.31%) 7.71s (± 0.23%) ~ 7.69s 7.74s p=0.570 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,355ms (± 0.68%) 2,350ms (± 0.83%) ~ 2,330ms 2,376ms p=0.574 n=6
Req 2 - geterr 5,416ms (± 1.41%) 5,473ms (± 1.65%) ~ 5,356ms 5,567ms p=0.336 n=6
Req 3 - references 326ms (± 0.98%) 324ms (± 0.36%) ~ 323ms 326ms p=0.413 n=6
Req 4 - navto 276ms (± 1.27%) 273ms (± 1.08%) ~ 270ms 278ms p=0.325 n=6
Req 5 - completionInfo count 1,356 (± 0.00%) 1,356 (± 0.00%) ~ 1,356 1,356 p=1.000 n=6
Req 5 - completionInfo 90ms (± 5.77%) 90ms (± 7.24%) ~ 77ms 95ms p=1.000 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,489ms (± 0.99%) 2,481ms (± 0.59%) ~ 2,466ms 2,503ms p=0.377 n=6
Req 2 - geterr 4,112ms (± 1.59%) 4,142ms (± 1.97%) ~ 4,082ms 4,253ms p=0.575 n=6
Req 3 - references 341ms (± 1.17%) 340ms (± 1.14%) ~ 334ms 343ms p=0.463 n=6
Req 4 - navto 284ms (± 0.18%) 286ms (± 0.85%) ~ 283ms 290ms p=0.276 n=6
Req 5 - completionInfo count 1,518 (± 0.00%) 1,518 (± 0.00%) ~ 1,518 1,518 p=1.000 n=6
Req 5 - completionInfo 87ms (± 5.63%) 87ms (± 5.69%) ~ 78ms 90ms p=0.492 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,599ms (± 0.88%) 2,603ms (± 0.76%) ~ 2,578ms 2,630ms p=1.000 n=6
Req 2 - geterr 1,680ms (± 2.42%) 1,728ms (± 1.72%) ~ 1,692ms 1,768ms p=0.066 n=6
Req 3 - references 112ms (± 9.77%) 115ms (± 8.86%) ~ 101ms 123ms p=0.935 n=6
Req 4 - navto 367ms (± 1.25%) 365ms (± 0.32%) ~ 363ms 366ms p=0.462 n=6
Req 5 - completionInfo count 2,073 (± 0.00%) 2,073 (± 0.00%) ~ 2,073 2,073 p=1.000 n=6
Req 5 - completionInfo 306ms (± 2.49%) 307ms (± 2.14%) ~ 297ms 313ms p=1.000 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • CompilerTSServer - node (v18.15.0, x64)
  • Compiler-UnionsTSServer - node (v18.15.0, x64)
  • xstateTSServer - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
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
Collaborator

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/56034/merge:

Something interesting changed - please have a look.

Details

Server exited prematurely with code unknown and signal SIGABRT 

Server exited prematurely with code unknown and signal SIGABRT

Affected repos

calcom/cal.com Raw error text: RepoResults7/calcom.cal.com.rawError.txt in the artifact folder

Last few requests

{"seq":855,"type":"request","command":"navto","arguments":{"searchValue":"a","maxResultCount":256}}
{"seq":856,"type":"request","command":"navto","arguments":{"searchValue":"a1R","maxResultCount":256}}
{"seq":857,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/apps/api/test/lib/middleware/httpMethods.test.ts"],"openFiles":[]}}
{"seq":858,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/apps/web/abTest/utils.ts","projectRootPath":"@PROJECT_ROOT@"}]}}

Repro steps

  1. git clone https://github.com/calcom/cal.com --recurse-submodules
  2. In dir cal.com, run git reset --hard fcc50c1d0f90d77253455e7caadab383a35ebefb
  3. In dir cal.com, run yarn install --no-immutable --mode=skip-build
  4. Back in the initial folder, download RepoResults7/calcom.cal.com.replay.txt from the artifact folder
  5. npm install --no-save @typescript/server-replay
  6. npx tsreplay ./cal.com ./calcom.cal.com.replay.txt path/to/tsserver.js
  7. npx tsreplay --help to learn about helpful switches for debugging, logging, etc

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/56034/merge:

Everything looks good!

@andrewbranch
Copy link
Member

Other note from the design meeting: was it intentional that we didn't mark the ImportAssertions global type as @deprecated? We think perhaps we just forgot.

@andrewbranch
Copy link
Member

Thanks @a-tarasyuk!

@andrewbranch andrewbranch merged commit 72d4973 into microsoft:main Jan 10, 2024
@a-tarasyuk a-tarasyuk mentioned this pull request Jan 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewbranch andrewbranch andrewbranch approved these changes

@jakebailey jakebailey jakebailey approved these changes

Assignees
No one assigned
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Type-check Import Attributes in static imports
4 participants
@a-tarasyuk @typescript-bot @jakebailey @andrewbranch