Don’t enforce export/declare overload modifier consistency across module augmentations #59416

andrewbranch · 2024-07-24T22:54:03Z

These rules are only there because it looks confusing to have overload declarations like

// what does this mean??
declare function foo(): any;
function foo(s: number): string;
export declare function foo(s: string): number;

But if the overloads came from a module augmentation, it's reasonable for them to have different modifiers (see code comment and tests for the differences that are allowed).

…ule augmentations

src/compiler/checker.ts

andrewbranch · 2024-07-25T18:18:02Z

@typescript-bot test it

typescript-bot · 2024-07-25T18:18:06Z

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 · 2024-07-25T18:41:19Z

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

Everything looks the same!

You can check the log here.

typescript-bot · 2024-07-25T18:43:43Z

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

Everything looks good!

gabritto · 2024-07-25T18:53:34Z

I feel like I don't have a good understanding of the old behavior to be able to assess the new one. Why was the old behavior in place? What are the bad scenarios we're trying to avoid, exactly?

typescript-bot · 2024-07-25T18:55:45Z

@andrewbranch
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	₃₀	₃₀	_~	_~	_~	_{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,638k (± 0.92%)}	_{192,326k (± 0.01%)}	_{-1,313k (- 0.68%)}	_192,305k	_192,353k	_{p=0.037 n=6}
_{Parse Time}	_{1.30s (± 1.33%)}	_{1.31s (± 0.62%)}	_~	_1.30s	_1.32s	_{p=0.402 n=6}
_{Bind Time}	_0.71s	_0.71s	_~	_~	_~	_{p=1.000 n=6}
_{Check Time}	_{9.56s (± 0.33%)}	_{9.58s (± 0.35%)}	_~	_9.52s	_9.62s	_{p=0.421 n=6}
_{Emit Time}	_{2.74s (± 0.64%)}	_{2.73s (± 0.43%)}	_~	_2.72s	_2.75s	_{p=0.164 n=6}
_{Total Time}	_{14.31s (± 0.26%)}	_{14.32s (± 0.22%)}	_~	_14.27s	_14.35s	_{p=0.520 n=6}
_{angular-1 - node (v18.15.0, x64)}
_Errors	₇	₇	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_945,532	_945,532	_~	_~	_~	_{p=1.000 n=6}
_Types	_409,507	_409,507	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{1,221,148k (± 0.00%)}	_{1,221,137k (± 0.00%)}	_~	_1,221,086k	_1,221,174k	_{p=0.689 n=6}
_{Parse Time}	_{6.60s (± 0.83%)}	_{6.60s (± 0.49%)}	_~	_6.56s	_6.64s	_{p=0.936 n=6}
_{Bind Time}	_{1.86s (± 0.44%)}	_{1.86s (± 0.45%)}	_~	_1.84s	_1.86s	_{p=0.673 n=6}
_{Check Time}	_{31.08s (± 0.34%)}	_{31.15s (± 0.26%)}	_~	_31.04s	_31.24s	_{p=0.230 n=6}
_{Emit Time}	_{15.03s (± 0.90%)}	_{15.03s (± 0.41%)}	_~	_14.96s	_15.11s	_{p=0.688 n=6}
_{Total Time}	_{54.56s (± 0.42%)}	_{54.63s (± 0.20%)}	_~	_54.49s	_54.77s	_{p=0.336 n=6}
_{mui-docs - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,433,165	_2,433,165	_~	_~	_~	_{p=1.000 n=6}
_Types	_1,002,715	_1,002,715	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,424,292k (± 0.00%)}	_{2,424,237k (± 0.00%)}	_~	_2,424,184k	_2,424,307k	_{p=0.065 n=6}
_{Parse Time}	_{8.43s (± 0.33%)}	_{8.43s (± 0.28%)}	_~	_8.40s	_8.46s	_{p=0.807 n=6}
_{Bind Time}	_{2.09s (± 0.36%)}	_{2.08s (± 0.47%)}	_~	_2.07s	_2.09s	_{p=0.300 n=6}
_{Check Time}	_{74.64s (± 0.56%)}	_{74.52s (± 0.44%)}	_~	_74.10s	_74.91s	_{p=0.688 n=6}
_{Emit Time}	_{0.28s (± 1.47%)}	_{0.28s (± 1.99%)}	_~	_0.27s	_0.28s	_{p=0.282 n=6}
_{Total Time}	_{85.43s (± 0.47%)}	_{85.30s (± 0.37%)}	_~	_84.89s	_85.68s	_{p=0.688 n=6}
_{self-build-src - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,230,046	_1,230,085	_{+39 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_265,645	_265,653	_{+8 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,347,399k (± 0.01%)}	_{2,347,135k (± 0.01%)}	_~	_2,346,802k	_2,347,640k	_{p=0.173 n=6}
_{Parse Time}	_{5.02s (± 0.60%)}	_{4.99s (± 0.87%)}	_~	_4.94s	_5.04s	_{p=0.336 n=6}
_{Bind Time}	_{1.90s (± 0.21%)}	_{1.89s (± 0.54%)}	_~	_1.88s	_1.90s	_{p=0.114 n=6}
_{Check Time}	_{34.70s (± 0.14%)}	_{34.71s (± 0.25%)}	_~	_34.61s	_34.82s	_{p=0.936 n=6}
_{Emit Time}	_{3.31s (± 0.47%)}	_{3.29s (± 0.95%)}	_~	_3.25s	_3.33s	_{p=0.261 n=6}
_{Total Time}	_{44.93s (± 0.18%)}	_{44.90s (± 0.29%)}	_~	_44.76s	_45.11s	_{p=0.378 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,230,046	_1,230,085	_{+39 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_265,645	_265,653	_{+8 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,422,113k (± 0.03%)}	_{2,421,618k (± 0.03%)}	_~	_2,420,523k	_2,422,469k	_{p=0.378 n=6}
_{Parse Time}	_{6.21s (± 0.67%)}	_{6.22s (± 0.94%)}	_~	_6.14s	_6.29s	_{p=0.575 n=6}
_{Bind Time}	_{2.05s (± 0.60%)}	_{2.04s (± 0.48%)}	_~	_2.03s	_2.05s	_{p=0.286 n=6}
_{Check Time}	_{41.41s (± 0.29%)}	_{41.46s (± 0.33%)}	_~	_41.34s	_41.72s	_{p=0.575 n=6}
_{Emit Time}	_{4.00s (± 1.42%)}	_{4.04s (± 0.92%)}	_~	_3.98s	_4.07s	_{p=0.298 n=6}
_{Total Time}	_{53.70s (± 0.34%)}	_{53.78s (± 0.32%)}	_~	_53.61s	_54.09s	_{p=0.470 n=6}
_{self-compiler - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_258,471	_258,510	_{+39 (+ 0.02%)}	_~	_~	_{p=0.001 n=6}
_Types	_105,534	_105,542	_{+8 (+ 0.01%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{429,159k (± 0.03%)}	_{429,096k (± 0.01%)}	_~	_429,057k	_429,126k	_{p=0.575 n=6}
_{Parse Time}	_{4.16s (± 0.62%)}	_{4.16s (± 0.47%)}	_~	_4.14s	_4.19s	_{p=1.000 n=6}
_{Bind Time}	_{1.64s (± 1.00%)}	_{1.63s (± 1.29%)}	_~	_1.61s	_1.66s	_{p=0.516 n=6}
_{Check Time}	_{22.27s (± 0.55%)}	_{22.27s (± 0.30%)}	_~	_22.16s	_22.35s	_{p=0.471 n=6}
_{Emit Time}	_{2.01s (± 0.81%)}	_{2.01s (± 1.31%)}	_~	_1.97s	_2.04s	_{p=1.000 n=6}
_{Total Time}	_{30.08s (± 0.53%)}	_{30.07s (± 0.27%)}	_~	_29.93s	_30.15s	_{p=0.630 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_Errors	₃₅	₃₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_224,931	_224,931	_~	_~	_~	_{p=1.000 n=6}
_Types	_94,146	_94,146	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{369,957k (± 0.01%)}	_{370,039k (± 0.03%)}	_~	_369,895k	_370,134k	_{p=0.230 n=6}
_{Parse Time}	_{2.77s (± 0.87%)}	_{2.78s (± 0.54%)}	_~	_2.76s	_2.79s	_{p=0.511 n=6}
_{Bind Time}	_{1.60s (± 1.00%)}	_{1.59s (± 0.86%)}	_~	_1.57s	_1.61s	_{p=0.251 n=6}
_{Check Time}	_{15.64s (± 0.36%)}	_{15.71s (± 0.24%)}	_~	_15.66s	_15.76s	_{p=0.054 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{20.01s (± 0.32%)}	_{20.07s (± 0.18%)}	_~	_20.01s	_20.11s	_{p=0.077 n=6}
_{vscode - node (v18.15.0, x64)}
_Errors	₁₁	₁₁	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,986,389	_2,986,389	_~	_~	_~	_{p=1.000 n=6}
_Types	_1,027,251	_1,027,251	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{3,110,393k (± 0.00%)}	_{3,110,447k (± 0.00%)}	_~	_3,110,409k	_3,110,493k	_{p=0.066 n=6}
_{Parse Time}	_{13.83s (± 0.29%)}	_{13.84s (± 0.14%)}	_~	_13.81s	_13.86s	_{p=0.871 n=6}
_{Bind Time}	_{4.29s (± 2.34%)}	_{4.37s (± 2.61%)}	_~	_4.25s	_4.48s	_{p=0.250 n=6}
_{Check Time}	_{78.84s (± 0.55%)}	_{78.51s (± 0.37%)}	_~	_78.08s	_78.88s	_{p=0.173 n=6}
_{Emit Time}	_{20.39s (± 0.49%)}	_{20.41s (± 0.42%)}	_~	_20.30s	_20.51s	_{p=0.747 n=6}
_{Total Time}	_{117.35s (± 0.44%)}	_{117.12s (± 0.20%)}	_~	_116.85s	_117.44s	_{p=0.575 n=6}
_{webpack - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_267,560	_267,560	_~	_~	_~	_{p=1.000 n=6}
_Types	_109,076	_109,076	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{412,428k (± 0.02%)}	_{412,405k (± 0.01%)}	_~	_412,381k	_412,435k	_{p=0.575 n=6}
_{Parse Time}	_{3.82s (± 0.52%)}	_{3.82s (± 0.49%)}	_~	_3.79s	_3.84s	_{p=0.684 n=6}
_{Bind Time}	_{1.71s (± 0.30%)}	_{1.71s (± 0.37%)}	_~	_1.70s	_1.72s	_{p=0.386 n=6}
_{Check Time}	_{16.88s (± 0.21%)}	_{16.83s (± 0.25%)}	_~	_16.77s	_16.88s	_{p=0.089 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{22.41s (± 0.19%)}	_{22.36s (± 0.19%)}	_~	_22.30s	_22.43s	_{p=0.126 n=6}
_{xstate-main - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_511,803	_511,803	_~	_~	_~	_{p=1.000 n=6}
_Types	_162,088	_162,088	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{449,272k (± 0.07%)}	_{448,989k (± 0.08%)}	_~	_448,627k	_449,384k	_{p=0.128 n=6}
_{Parse Time}	_{3.90s (± 0.53%)}	_{3.90s (± 0.72%)}	_~	_3.87s	_3.94s	_{p=1.000 n=6}
_{Bind Time}	_{1.44s (± 1.11%)}	_{1.44s (± 0.95%)}	_~	_1.42s	_1.46s	_{p=1.000 n=6}
_{Check Time}	_{21.21s (± 0.37%)}	_{21.28s (± 0.43%)}	_~	_21.20s	_21.41s	_{p=0.226 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{26.55s (± 0.22%)}	_{26.62s (± 0.29%)}	_~	_26.53s	_26.69s	_{p=0.090 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	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

andrewbranch · 2024-07-25T19:06:28Z

Stating my understanding, but I might have some of the details wrong. AFAIK, overloads are additional declarations of the same method/function symbol. Since there’s ultimately only one merged symbol, all these modifiers necessarily apply uniformly to the symbol. We don’t really have mechanisms in place to reason about what it would mean for some signatures to be exported while others aren’t. And the presence or absence of declare on an overload signature doesn’t really have any meaning at all, since overloads aren’t a runtime/emitting construct anyway. So I think the rules are in place because the mixing of modifiers like

export function foo(x: T1): T2;
export function foo(x: T3): T4;
       function foo(x: T5): T6;
export function foo(x: any): any {
  // ...
}

looks like it might make the third overload file-local, but there’s actually nothing in place to make that happen, so we want to enforce consistency in the modifiers.

typescript-bot · 2024-07-25T20:01:38Z

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

Everything looks good!

Don’t enforce export/declare overload modifier consistency across mod…

1003852

…ule augmentations

typescript-bot assigned andrewbranch Jul 24, 2024

typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Jul 24, 2024

andrewbranch requested review from sandersn and gabritto July 24, 2024 22:55

gabritto reviewed Jul 25, 2024

View reviewed changes

src/compiler/checker.ts Show resolved Hide resolved

andrewbranch added 3 commits July 25, 2024 08:46

Update other baseline

4195f12

Hoist canonicalFlags computation

de522ae

Update other baseline

fb50670

gabritto approved these changes Jul 25, 2024

View reviewed changes

andrewbranch merged commit 9405f21 into microsoft:main Jul 29, 2024
32 checks passed

andrewbranch deleted the bug/58756 branch July 29, 2024 16:20

eps1lon mentioned this pull request Jul 29, 2024

Fix next/navigation type augmentation vercel/next.js#66489

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don’t enforce export/declare overload modifier consistency across module augmentations #59416

Don’t enforce export/declare overload modifier consistency across module augmentations #59416

andrewbranch commented Jul 24, 2024 •

edited

Loading

andrewbranch commented Jul 25, 2024

typescript-bot commented Jul 25, 2024 •

edited

Loading

typescript-bot commented Jul 25, 2024

typescript-bot commented Jul 25, 2024

gabritto commented Jul 25, 2024

typescript-bot commented Jul 25, 2024

tsc

andrewbranch commented Jul 25, 2024

typescript-bot commented Jul 25, 2024

Don’t enforce export/declare overload modifier consistency across module augmentations #59416

Don’t enforce export/declare overload modifier consistency across module augmentations #59416

Conversation

andrewbranch commented Jul 24, 2024 • edited Loading

andrewbranch commented Jul 25, 2024

typescript-bot commented Jul 25, 2024 • edited Loading

typescript-bot commented Jul 25, 2024

typescript-bot commented Jul 25, 2024

gabritto commented Jul 25, 2024

typescript-bot commented Jul 25, 2024

tsc

andrewbranch commented Jul 25, 2024

typescript-bot commented Jul 25, 2024

andrewbranch commented Jul 24, 2024 •

edited

Loading

typescript-bot commented Jul 25, 2024 •

edited

Loading