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

Infer from union combined single signatures #58482

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented May 9, 2024

I treat this as an experiment right now. I think the logic is sound and it makes sense. However, I wonder:

  • if it does impact top repos/DT in a negative way? are we missing some tests?
  • what's the perf impact of this? that said, if there would be a meaningful perf impact here there still might be ways to improve this

fixes #58468

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 9, 2024
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@@ -12,8 +12,8 @@ declare const f: { (x: string): number, a: "" } & { a: number }
> : ^^^^^^

f()
>f() : any
> : ^^^
>f() : number
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function itself is still not callable here but that erroneous call shows a better return type now. This is caused by getSingleSignature handling structured types now (an intersection in this very case)

@@ -280,5 +280,5 @@ export class L {
set x(value: any);
}
export class M {
set x(value: any);
set x(value: boolean);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comes from:

/**
 * @param {number | boolean} v
 */
const mSetter1 = (v) => {}
/**
 * @param {string | boolean} v
 */
const mSetter2 = (v) => {}
export class M {}
Object.defineProperty(M.prototype, "x", {
    set: Math.random() ? mSetter1 : mSetter2
});

This feels like an improvement, those setter functions can be combined safely to a single signature with a boolean parameter

@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 9, 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 @jakebailey, the results of running the DT tests are ready.

There were interesting changes:

Branch only errors:

Package: webpack/v4
Error:

Error: 
/mnt/vss/_work/1/DefinitelyTyped/types/webpack/v4/index.d.ts
  1550:9  error  TypeScript@local compile error: 
Property 'run' in type 'Compiler' is not assignable to the same property in base type 'ICompiler'.
  Type '(handler: Handler) => void' is not assignable to type '(handler: Handler | MultiHandler) => void'.
    Types of parameters 'handler' and 'handler' are incompatible.
      Types of parameters 'stats' and 'stats' are incompatible.
        Type 'Stats' is not assignable to type 'Stats & MultiStats'.
          Property 'stats' is missing in type 'Stats' but required in type 'MultiStats'                                                                             @definitelytyped/expect
  1551:9  error  TypeScript@local compile error: 
Property 'watch' in type 'Compiler' is not assignable to the same property in base type 'ICompiler'.
  Type '(watchOptions: WatchOptions, handler: Handler) => Watching' is not assignable to type '(watchOptions: WatchOptions, handler: Handler | MultiHandler) => Watching'.
    Types of parameters 'handler' and 'handler' are incompatible.
      Types of parameters 'stats' and 'stats' are incompatible.
        Type 'Stats' is not assignable to type 'Stats & MultiStats'                                                                                                    @definitelytyped/expect
  1575:9  error  TypeScript@local compile error: 
Property 'run' in type 'MultiCompiler' is not assignable to the same property in base type 'ICompiler'.
  Type '(handler: MultiHandler) => void' is not assignable to type '(handler: Handler | MultiHandler) => void'.
    Types of parameters 'handler' and 'handler' are incompatible.
      Types of parameters 'stats' and 'stats' are incompatible.
        Type 'MultiStats' is not assignable to type 'Stats & MultiStats'.
          Type 'MultiStats' is missing the following properties from type 'Stats': compilation, formatFilePath, normalizeFieldKey, sortOrderRegular  @definitelytyped/expect
  1576:9  error  TypeScript@local compile error: 
Property 'watch' in type 'MultiCompiler' is not assignable to the same property in base type 'ICompiler'.
  Type '(watchOptions: WatchOptions, handler: MultiHandler) => MultiWatching' is not assignable to type '(watchOptions: WatchOptions, handler: Handler | MultiHandler) => Watching'.
    Types of parameters 'handler' and 'handler' are incompatible.
      Types of parameters 'stats' and 'stats' are incompatible.
        Type 'MultiStats' is not assignable to type 'Stats & MultiStats'                                                                                @definitelytyped/expect

✖ 4 problems (4 errors, 0 warnings)

    at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240509/node_modules/@definitelytyped/dtslint/dist/index.js:194:28)
    at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240509/node_modules/@definitelytyped/dtslint/dist/index.js:186:20)

You can check the log here.

@typescript-bot
Copy link
Collaborator

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

Something interesting changed - please have a look.

Details

fp-ts

tsconfig.json

  • [MISSING] error TS2322: Type 'ReaderTaskEither<{ a: string; }, string, void>' is not assignable to type 'ReaderTaskEither<{ a: number; }, string, any>'.

tsconfig.eslint.json

  • [MISSING] error TS2322: Type 'ReaderTaskEither<{ a: string; }, string, void>' is not assignable to type 'ReaderTaskEither<{ a: number; }, string, any>'.

dtslint/ts3.5/tsconfig.json

  • [NEW] error TS2322: Type 'ReaderTaskEither<{ a: number; }, string, number>' is not assignable to type 'ReaderTaskEither<{ a: number; }, boolean, any>'.
  • [MISSING] error TS2322: Type 'ReaderTaskEither<{ a: number; }, boolean, void>' is not assignable to type 'ReaderTaskEither<{ a: number; }, string, any>'.
  • [MISSING] error TS2322: Type 'ReaderTaskEither<{ a: string; }, string, void>' is not assignable to type 'ReaderTaskEither<{ a: number; }, string, any>'.

webpack

tsconfig.types.json

  • [NEW] error TS2684: The 'this' context of type 'Function | WebpackPluginFunction' is not assignable to method's 'this' of type 'Function & ((this: Compiler, compiler: Compiler) => any)'.
  • [MISSING] error TS2684: The 'this' context of type 'Function | WebpackPluginFunction' is not assignable to method's 'this' of type 'Function & ((this: Compiler, compiler: Compiler) => void)'.

@typescript-bot
Copy link
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 31 +1 (+ 3.33%) ~ ~ p=0.001 n=6
Symbols 62,154 67,077 🔻+4,923 (+ 7.92%) ~ ~ p=0.001 n=6
Types 50,248 52,257 +2,009 (+ 4.00%) ~ ~ p=0.001 n=6
Memory used 193,412k (± 0.94%) 198,358k (± 0.07%) +4,946k (+ 2.56%) 198,271k 198,644k p=0.005 n=6
Parse Time 1.30s (± 1.05%) 1.29s (± 1.30%) ~ 1.27s 1.31s p=0.621 n=6
Bind Time 0.72s 0.72s ~ ~ ~ p=1.000 n=6
Check Time 9.53s (± 0.34%) 9.69s (± 0.34%) +0.16s (+ 1.63%) 9.65s 9.73s p=0.005 n=6
Emit Time 2.64s (± 0.24%) 2.56s (± 0.73%) 🟩-0.08s (- 3.22%) 2.54s 2.59s p=0.004 n=6
Total Time 14.19s (± 0.27%) 14.24s (± 0.23%) +0.06s (+ 0.40%) 14.20s 14.29s p=0.037 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,110 944,137 +27 (+ 0.00%) ~ ~ p=0.001 n=6
Types 407,141 407,122 -19 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 1,221,977k (± 0.01%) 1,222,243k (± 0.01%) +266k (+ 0.02%) 1,222,156k 1,222,329k p=0.005 n=6
Parse Time 6.78s (± 0.59%) 6.81s (± 0.56%) ~ 6.74s 6.85s p=0.197 n=6
Bind Time 1.88s (± 0.52%) 1.88s (± 0.43%) ~ 1.87s 1.89s p=0.604 n=6
Check Time 31.11s (± 0.30%) 31.32s (± 0.21%) +0.21s (+ 0.68%) 31.22s 31.39s p=0.013 n=6
Emit Time 14.79s (± 0.52%) 14.75s (± 0.61%) ~ 14.62s 14.84s p=0.575 n=6
Total Time 54.56s (± 0.29%) 54.76s (± 0.20%) ~ 54.56s 54.87s p=0.066 n=6
mui-docs - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 1,950,503 1,950,644 +141 (+ 0.01%) ~ ~ p=0.001 n=6
Types 677,995 678,049 +54 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 1,753,162k (± 0.00%) 1,754,054k (± 0.00%) +892k (+ 0.05%) 1,754,023k 1,754,090k p=0.005 n=6
Parse Time 6.66s (± 0.31%) 6.66s (± 0.12%) ~ 6.65s 6.67s p=0.402 n=6
Bind Time 2.29s (± 0.36%) 2.29s (± 0.23%) ~ 2.28s 2.29s p=0.752 n=6
Check Time 56.15s (± 0.51%) 56.35s (± 0.19%) ~ 56.15s 56.47s p=0.229 n=6
Emit Time 0.14s 0.14s (± 4.51%) ~ 0.13s 0.15s p=1.000 n=6
Total Time 65.23s (± 0.47%) 65.44s (± 0.17%) ~ 65.24s 65.57s p=0.297 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,220,247 1,221,030 +783 (+ 0.06%) ~ ~ p=0.001 n=6
Types 258,932 259,000 +68 (+ 0.03%) ~ ~ p=0.001 n=6
Memory used 2,334,782k (± 0.03%) 2,335,084k (± 0.02%) ~ 2,334,616k 2,335,592k p=0.230 n=6
Parse Time 5.00s (± 0.92%) 5.01s (± 0.89%) ~ 4.94s 5.06s p=0.630 n=6
Bind Time 1.87s (± 0.71%) 1.87s (± 0.86%) ~ 1.86s 1.90s p=0.866 n=6
Check Time 33.43s (± 0.35%) 33.38s (± 0.19%) ~ 33.31s 33.45s p=0.575 n=6
Emit Time 2.63s (± 2.48%) 2.65s (± 0.97%) ~ 2.62s 2.69s p=0.810 n=6
Total Time 42.96s (± 0.28%) 42.93s (± 0.22%) ~ 42.79s 43.02s 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,220,247 1,221,030 +783 (+ 0.06%) ~ ~ p=0.001 n=6
Types 258,932 259,000 +68 (+ 0.03%) ~ ~ p=0.001 n=6
Memory used 2,410,495k (± 0.03%) 2,410,866k (± 0.02%) ~ 2,410,426k 2,411,450k p=0.575 n=6
Parse Time 6.21s (± 1.44%) 6.22s (± 1.19%) ~ 6.14s 6.32s p=0.873 n=6
Bind Time 2.03s (± 1.01%) 2.01s (± 0.86%) ~ 1.99s 2.04s p=0.195 n=6
Check Time 39.80s (± 0.36%) 39.85s (± 0.16%) ~ 39.75s 39.91s p=0.471 n=6
Emit Time 3.13s (± 2.83%) 3.16s (± 2.34%) ~ 3.03s 3.25s p=0.687 n=6
Total Time 51.18s (± 0.26%) 51.25s (± 0.27%) ~ 51.02s 51.42s p=0.378 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 256,476 256,665 +189 (+ 0.07%) ~ ~ p=0.001 n=6
Types 104,239 104,261 +22 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 425,301k (± 0.01%) 425,466k (± 0.01%) +165k (+ 0.04%) 425,433k 425,538k p=0.005 n=6
Parse Time 4.17s (± 0.30%) 4.19s (± 0.33%) ~ 4.16s 4.20s p=0.097 n=6
Bind Time 1.61s (± 0.39%) 1.61s (± 0.51%) ~ 1.60s 1.62s p=0.432 n=6
Check Time 21.93s (± 0.29%) 21.85s (± 0.38%) ~ 21.74s 21.99s p=0.077 n=6
Emit Time 1.72s (± 1.25%) 1.73s (± 1.12%) ~ 1.70s 1.75s p=0.746 n=6
Total Time 29.43s (± 0.20%) 29.37s (± 0.27%) ~ 29.27s 29.51s p=0.102 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,575 224,885 +310 (+ 0.14%) ~ ~ p=0.001 n=6
Types 93,785 93,816 +31 (+ 0.03%) ~ ~ p=0.001 n=6
Memory used 369,781k (± 0.02%) 369,981k (± 0.03%) +200k (+ 0.05%) 369,860k 370,103k p=0.008 n=6
Parse Time 2.82s (± 1.13%) 2.83s (± 0.75%) ~ 2.80s 2.86s p=0.514 n=6
Bind Time 1.58s (± 1.09%) 1.58s (± 0.87%) ~ 1.56s 1.60s p=0.622 n=6
Check Time 15.56s (± 0.23%) 15.57s (± 0.14%) ~ 15.54s 15.60s p=0.366 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 19.97s (± 0.15%) 19.98s (± 0.10%) ~ 19.96s 20.01s p=0.280 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,809,620 2,810,338 +718 (+ 0.03%) ~ ~ p=0.001 n=6
Types 953,428 954,156 +728 (+ 0.08%) ~ ~ p=0.001 n=6
Memory used 2,981,435k (± 0.00%) 2,982,731k (± 0.00%) +1,296k (+ 0.04%) 2,982,545k 2,982,861k p=0.005 n=6
Parse Time 16.94s (± 0.60%) 16.97s (± 0.28%) ~ 16.89s 17.01s p=0.573 n=6
Bind Time 5.03s (± 0.66%) 5.03s (± 0.30%) ~ 5.01s 5.04s p=1.000 n=6
Check Time 90.41s (± 3.41%) 91.28s (± 3.34%) ~ 88.38s 94.17s p=0.575 n=6
Emit Time 27.41s (± 8.58%) 26.74s (± 9.75%) ~ 24.13s 29.38s p=1.000 n=6
Total Time 139.79s (± 0.59%) 140.02s (± 0.36%) ~ 139.44s 140.82s p=0.471 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 265,866 266,143 +277 (+ 0.10%) ~ ~ p=0.001 n=6
Types 108,424 108,439 +15 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 410,602k (± 0.02%) 410,705k (± 0.01%) +103k (+ 0.03%) 410,653k 410,818k p=0.030 n=6
Parse Time 4.77s (± 0.58%) 4.74s (± 0.51%) ~ 4.71s 4.77s p=0.126 n=6
Bind Time 2.08s (± 0.50%) 2.06s (± 0.96%) -0.03s (- 1.44%) 2.02s 2.08s p=0.012 n=6
Check Time 20.92s (± 0.41%) 20.92s (± 0.16%) ~ 20.88s 20.98s p=0.334 n=6
Emit Time 0.00s 0.00s (±244.70%) ~ 0.00s 0.01s p=0.405 n=6
Total Time 27.77s (± 0.39%) 27.71s (± 0.19%) ~ 27.66s 27.81s p=0.173 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 524,576 527,689 +3,113 (+ 0.59%) ~ ~ p=0.001 n=6
Types 178,847 178,877 +30 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 462,591k (± 0.03%) 463,877k (± 0.02%) +1,286k (+ 0.28%) 463,778k 464,092k p=0.005 n=6
Parse Time 3.11s (± 0.85%) 3.13s (± 1.42%) ~ 3.07s 3.18s p=0.687 n=6
Bind Time 1.17s (± 1.26%) 1.17s (± 1.30%) ~ 1.15s 1.19s p=0.507 n=6
Check Time 18.24s (± 0.57%) 18.32s (± 0.68%) ~ 18.10s 18.44s p=0.128 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.53s (± 0.40%) 22.61s (± 0.54%) ~ 22.41s 22.76s p=0.258 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

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

Something interesting changed - please have a look.

Details

immich-app/immich

2 of 7 projects failed to build with the old tsc and were ignored

server/tsconfig.json

  • error TS2322: Type 'Mock<[event: E, userId: string, data: ClientEventMap[E]], void>' is not assignable to type 'MockInstance<[event: keyof ClientEventMap, userId: string, data: string | string[] | AssetResponseDto | ServerVersionResponseDto | ReleaseNotification | Record<...>], void> & (<E extends keyof ClientEventMap>(event: E, userId: string, data: ClientEventMap[E]) => void)'.
  • error TS2322: Type 'Mock<[event: E, data: ClientEventMap[E]], void>' is not assignable to type 'MockInstance<[event: keyof ClientEventMap, data: string | string[] | AssetResponseDto | ServerVersionResponseDto | ReleaseNotification | Record<...>], void> & (<E extends keyof ClientEventMap>(event: E, data: ClientEventMap[E]) => void)'.
  • error TS2322: Type 'Mock<[event: E, data: ServerEventMap[E]], boolean>' is not assignable to type 'MockInstance<[event: keyof ServerEventMap, data: { userId: string; } | null], boolean> & (<E extends keyof ServerEventMap>(event: E, data: ServerEventMap[E]) => boolean)'.
  • error TS2322: Type 'Mock<[event: E, data: ServerAsyncEventMap[E]], Promise<any>>' is not assignable to type 'MockInstance<[event: ServerAsyncEvent.CONFIG_VALIDATE, data: { newConfig: SystemConfig; oldConfig: SystemConfig; }], Promise<any>> & (<E extends ServerAsyncEvent.CONFIG_VALIDATE>(event: E, data: ServerAsyncEventMap[E]) => Promise<...>)'.
  • error TS2322: Type 'Mock<[key: T, value: SystemMetadata[T]], Promise<void>>' is not assignable to type 'MockInstance<[key: keyof SystemMetadata, value: { lastUpdate?: string | undefined; lastImportFileName?: string | undefined; } | { isOnboarded: boolean; }], Promise<void>> & (<T extends keyof SystemMetadata>(key: T, value: SystemMetadata[T]) => Promise<...>)'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.call on an union with different arguments counts fails when passing any
3 participants