Use symbols of type aliases when emitting declarations #56087

Andarist · 2023-10-12T18:51:48Z

jakebailey · 2023-10-12T19:50:42Z

src/compiler/checker.ts

@@ -5665,6 +5665,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
            if (getSymbolIfSameReference(exported, symbol)) {
                return exported;
            }
+            if (symbol.flags & SymbolFlags.TypeAlias && exported.declarations?.find(isTypeAlias)) {


I think we canonically use our own find helper here to avoid the this penalty; might be important if this is hot code.

I have literally copied this bit from getDeclaredTypeOfTypeAlias :P
https://github.dev/microsoft/TypeScript/blob/cf8763b6c3193deefd095c86ce79921f23de307b/src/compiler/checker.ts#L12436

Should I switch both call sites to ur custom find?

Hah, then nevermind.

to avoid the this penalty

The original reason was that find wasn't defined in old runtimes. What is the this penalty? Maybe that's a good replacement reason.

IIRC functions passed to find and other helpers like it are bound such that this in them points to the array.

weswigham · 2023-11-06T23:09:27Z

src/compiler/checker.ts

@@ -5665,6 +5665,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
            if (getSymbolIfSameReference(exported, symbol)) {
                return exported;
            }
+            if (symbol.flags & SymbolFlags.TypeAlias && exported.declarations?.find(isTypeAlias)) {
+                const aliasSymbol = getDeclaredTypeOfTypeAlias(exported).aliasSymbol;
+                if (aliasSymbol && getSymbolIfSameReference(aliasSymbol, symbol)) {


Shouldn't this be a change in the logic of getSymbolIfSameReference instead? To make a type alias that points directly at another type count as a "same reference" of that type? It's been reused in one or two other places, but it basically only exists to answer "should we consider these two symbols as ultimately referring to the same thing", and mostly just for the usages here, in this function. In particular, only doing this check here means 1. the fastpath lookup will fail for type aliases that pass this check, so they always hit the slow path and 2. an export ='d type alias will never be considered equal to the target symbol.

weswigham · 2023-11-07T18:41:34Z

src/compiler/checker.ts

@@ -5672,6 +5672,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
     * Checks if two symbols, through aliasing and/or merging, refer to the same thing
     */
    function getSymbolIfSameReference(s1: Symbol, s2: Symbol) {
+        if (s1.flags & SymbolFlags.TypeAlias && s2.declarations?.find(isTypeAlias)) {


Maybe extract this into another helper function? Seems a bit repetitive.

This checks both s1 and s2 and I don't have any good name for a helper function that would accept both as arguments or other ideas on how to improve this. Everything I have tried locally felt clunky to me.

weswigham · 2024-01-09T19:15:51Z

@typescript-bot test this
@typescript-bot run dt
@typescript-bot test top100
@typescript-bot perf test public

typescript-bot · 2024-01-09T19:15:54Z

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

Update: The results are in!

typescript-bot · 2024-01-09T19:15:55Z

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

Update: The results are in!

typescript-bot · 2024-01-09T19:15:55Z

Heya @weswigham, I've started to run the public perf test suite on this PR at 862603b. You can monitor the build here.

Update: The results are in!

typescript-bot · 2024-01-09T19:59:54Z

Hey @weswigham, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

typescript-bot · 2024-01-09T20:04:28Z

@weswigham
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
_{self-build-src - node (v20.5.1, x64)}
_{Memory used}	_{2,739,302k (± 5.70%)}	_{2,681,294k (± 5.15%)}	_~	_2,573,910k	_2,922,103k	_{p=0.810 n=6}
_{Parse Time}	_{5.03s (± 1.37%)}	_{5.02s (± 0.82%)}	_~	_4.95s	_5.07s	_{p=0.687 n=6}
_{Bind Time}	_{1.99s (± 0.76%)}	_{2.00s (± 0.88%)}	_~	_1.97s	_2.02s	_{p=0.250 n=6}
_{Check Time}	_{31.97s (± 0.51%)}	_{31.97s (± 0.24%)}	_~	_31.87s	_32.10s	_{p=0.873 n=6}
_{Emit Time}	_{2.76s (± 4.13%)}	_{2.85s (± 3.75%)}	_~	_2.71s	_3.02s	_{p=0.230 n=6}
_{Total Time}	_{41.77s (± 0.42%)}	_{41.84s (± 0.37%)}	_~	_41.67s	_42.08s	_{p=0.575 n=6}
_{self-compiler - node (v20.5.1, x64)}
_{Memory used}	_{418,834k (± 0.01%)}	_{418,888k (± 0.01%)}	_~	_418,792k	_418,937k	_{p=0.093 n=6}
_{Parse Time}	_{2.89s (± 0.81%)}	_{2.88s (± 0.38%)}	_~	_2.86s	_2.89s	_{p=0.934 n=6}
_{Bind Time}	_{1.13s (± 0.46%)}	_{1.14s (± 0.96%)}	_~	_1.13s	_1.16s	_{p=0.247 n=6}
_{Check Time}	_{14.07s (± 0.37%)}	_{14.13s (± 0.51%)}	_~	_14.07s	_14.25s	_{p=0.128 n=6}
_{Emit Time}	_{1.05s (± 1.53%)}	_{1.05s (± 1.47%)}	_~	_1.04s	_1.08s	_{p=0.718 n=6}
_{Total Time}	_{19.14s (± 0.24%)}	_{19.21s (± 0.42%)}	_~	_19.13s	_19.34s	_{p=0.170 n=6}
_{vscode - node (v20.5.1, x64)}
_{Memory used}	_{2,828,077k (± 0.00%)}	_{2,828,057k (± 0.00%)}	_~	_2,828,032k	_2,828,106k	_{p=0.128 n=6}
_{Parse Time}	_{10.73s (± 0.14%)}	_{10.74s (± 0.21%)}	_~	_10.71s	_10.77s	_{p=0.292 n=6}
_{Bind Time}	_{3.43s (± 0.51%)}	_{3.44s (± 0.80%)}	_~	_3.42s	_3.48s	_{p=0.618 n=6}
_{Check Time}	_{56.12s (± 0.50%)}	_{56.12s (± 0.40%)}	_~	_55.86s	_56.37s	_{p=1.000 n=6}
_{Emit Time}	_{16.20s (± 0.34%)}	_{16.18s (± 0.45%)}	_~	_16.10s	_16.27s	_{p=0.376 n=6}
_{Total Time}	_{86.48s (± 0.36%)}	_{86.48s (± 0.28%)}	_~	_86.16s	_86.78s	_{p=0.873 n=6}

System info unknown

Hosts

node (v20.5.1, x64)

Scenarios

self-build-src - node (v20.5.1, x64)
self-compiler - node (v20.5.1, x64)
vscode - node (v20.5.1, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2024-01-09T20:20:22Z

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

Something interesting changed - please have a look.

Details

chakra-ui/chakra-ui

4 of 28 projects failed to build with the old tsc and were ignored

packages/components/tsconfig.build.json

error TS5056: Cannot write file '/mnt/ts_downloads/chakra-ui/packages/components/dist/types/menu/menu.stories.d.ts' because it would be overwritten by multiple input files.
- Project Scope

jakebailey · 2024-03-19T17:55:34Z

I've sent a revert for this in #57849; this PR caused two bugs and a perf regression, so I think it's safest to back it out as it was just to fix a bug that has existed seemingly forever.

Use symbols of type aliases when emitting declarations

0068b37

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 12, 2023

fixed crash

672e797

jakebailey reviewed Oct 12, 2023

View reviewed changes

sandersn added this to Not started in PR Backlog Oct 25, 2023

sandersn requested a review from weswigham November 3, 2023 17:31

sandersn assigned weswigham Nov 3, 2023

sandersn moved this from Not started to Waiting on reviewers in PR Backlog Nov 3, 2023

PR Backlog automation moved this from Waiting on reviewers to Waiting on author Nov 6, 2023

weswigham requested changes Nov 6, 2023

View reviewed changes

Move the fix to getSymbolIfSameReference itself

862603b

Andarist requested a review from weswigham November 7, 2023 08:35

weswigham reviewed Nov 7, 2023

View reviewed changes

Andarist requested a review from weswigham December 28, 2023 23:26

PR Backlog automation moved this from Waiting on author to Needs merge Jan 9, 2024

weswigham approved these changes Jan 9, 2024

View reviewed changes

weswigham merged commit 4bcbc16 into microsoft:main Jan 9, 2024
19 checks passed

PR Backlog automation moved this from Needs merge to Done Jan 9, 2024

RyanCavanaugh mentioned this pull request Feb 9, 2024

'Circular definition of import alias' error in 5.4.0-Beta #57357

Closed

Andarist mentioned this pull request Feb 27, 2024

Fixed too eager type resolution when looking for symbol aliases #57396

Open

MichaelMitchell-at mentioned this pull request Mar 14, 2024

Performance regression in TS 5.4 #57781

Closed

bradzacher mentioned this pull request Mar 19, 2024

[regression - 5.4] Generated .d.ts file contains invalid types #57843

Closed

jakebailey mentioned this pull request Mar 19, 2024

Revert PR 56087 #57849

Merged

jakebailey mentioned this pull request Mar 19, 2024

Portability error issued when reachable source file is redirected to an unreachable target #49171

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use symbols of type aliases when emitting declarations #56087

Use symbols of type aliases when emitting declarations #56087

Andarist commented Oct 12, 2023

jakebailey Oct 12, 2023

Andarist Oct 12, 2023

jakebailey Oct 12, 2023

sandersn Nov 3, 2023 •

edited

Loading

jakebailey Nov 3, 2023

weswigham Nov 6, 2023

weswigham Nov 7, 2023

Andarist Dec 28, 2023

weswigham commented Jan 9, 2024

typescript-bot commented Jan 9, 2024 •

edited

Loading

typescript-bot commented Jan 9, 2024 •

edited

Loading

typescript-bot commented Jan 9, 2024 •

edited

Loading

typescript-bot commented Jan 9, 2024

typescript-bot commented Jan 9, 2024

tsc

typescript-bot commented Jan 9, 2024

chakra-ui/chakra-ui

packages/components/tsconfig.build.json

jakebailey commented Mar 19, 2024

Use symbols of type aliases when emitting declarations #56087

Use symbols of type aliases when emitting declarations #56087

Conversation

Andarist commented Oct 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weswigham commented Jan 9, 2024

typescript-bot commented Jan 9, 2024 • edited Loading

typescript-bot commented Jan 9, 2024 • edited Loading

typescript-bot commented Jan 9, 2024 • edited Loading

typescript-bot commented Jan 9, 2024

typescript-bot commented Jan 9, 2024

tsc

typescript-bot commented Jan 9, 2024

jakebailey commented Mar 19, 2024

sandersn Nov 3, 2023 •

edited

Loading

typescript-bot commented Jan 9, 2024 •

edited

Loading

typescript-bot commented Jan 9, 2024 •

edited

Loading

typescript-bot commented Jan 9, 2024 •

edited

Loading