Emit declarations using alternative containing modules for types exported using separate export statements #56857

Andarist · 2023-12-23T12:16:15Z

…rted using separate export statements

Andarist · 2023-12-23T12:18:12Z

src/compiler/checker.ts

+            return undefined;
+        }
+        const containers = mapDefined(candidates, candidate => getAliasForSymbolInContainer(candidate, symbol) ? candidate : undefined);
+        return containers.length === 1 ? getWithAlternativeContainers(containers[0]) : containers;


this line is ad-hoc, perhaps I should just flatMap this. From what I've seen the consumers of this return value assume some ordering - or well, operate on first items with priority or smth. So I'm not sure how it would behave in more complex scenarios with smth like: [...withAlternativeContainersA, ...withAlternativeContainersB, ...withAlternativeContainersC]

Wait I’m confused; why would there only being a single element in the array affect whether you need to repackage it or not?

I tried to explain this in the comment above. This is the only case I know how to reason about and about which i’m somewhat confident here. This is not the final solution - i’m seeking guidance what should be done here when there are more items in this array

From what I've seen the consumers of this return value assume some ordering - or well, operate on first items with priority or smth

Indeed, the results returned here are intended to be in priority order for "what is best to serialize", which is, admittedly, a bit subjective. IMO, the ordering here would probably be the direct symbols for each available container symbol, followed by all their alternative symbols, in order - so not quite a flat map.

I implemented the suggested solution. Could you take another look?

weswigham · 2024-01-02T21:30:37Z

src/compiler/checker.ts

+            return undefined;
+        }
+        const containers = mapDefined(candidates, candidate => getAliasForSymbolInContainer(candidate, symbol) ? candidate : undefined);
+        return containers.length === 1 ? getWithAlternativeContainers(containers[0]) : containers;


From what I've seen the consumers of this return value assume some ordering - or well, operate on first items with priority or smth

Indeed, the results returned here are intended to be in priority order for "what is best to serialize", which is, admittedly, a bit subjective. IMO, the ordering here would probably be the direct symbols for each available container symbol, followed by all their alternative symbols, in order - so not quite a flat map.

weswigham · 2024-01-09T18:34:26Z

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

typescript-bot · 2024-01-09T18:34:29Z

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

Update: The results are in!

typescript-bot · 2024-01-09T18:34:29Z

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

Update: The results are in!

typescript-bot · 2024-01-09T18:34:29Z

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

Update: The results are in!

typescript-bot · 2024-01-09T19:19:53Z

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-09T19:27:31Z

@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,265k (± 5.68%)}	_{2,656,870k (± 5.35%)}	_~	_2,574,037k	_2,921,340k	_{p=0.230 n=6}
_{Parse Time}	_{5.01s (± 0.85%)}	_{5.01s (± 0.43%)}	_~	_4.99s	_5.04s	_{p=0.748 n=6}
_{Bind Time}	_{1.97s (± 1.09%)}	_{1.99s (± 1.04%)}	_~	_1.96s	_2.02s	_{p=0.517 n=6}
_{Check Time}	_{32.02s (± 0.31%)}	_{32.02s (± 0.55%)}	_~	_31.72s	_32.26s	_{p=0.936 n=6}
_{Emit Time}	_{2.80s (± 2.69%)}	_{2.82s (± 3.69%)}	_~	_2.68s	_2.99s	_{p=0.936 n=6}
_{Total Time}	_{41.83s (± 0.21%)}	_{41.86s (± 0.16%)}	_~	_41.76s	_41.94s	_{p=0.471 n=6}
_{self-compiler - node (v20.5.1, x64)}
_{Memory used}	_{418,813k (± 0.02%)}	_{418,829k (± 0.02%)}	_~	_418,755k	_418,981k	_{p=0.377 n=6}
_{Parse Time}	_{2.89s (± 0.75%)}	_{2.90s (± 0.68%)}	_~	_2.87s	_2.92s	_{p=0.871 n=6}
_{Bind Time}	_{1.13s (± 0.48%)}	_{1.13s (± 0.48%)}	_~	_1.13s	_1.14s	_{p=1.000 n=6}
_{Check Time}	_{14.09s (± 0.22%)}	_{14.04s (± 0.40%)}	_~	_13.96s	_14.11s	_{p=0.077 n=6}
_{Emit Time}	_{1.04s (± 0.78%)}	_{1.04s (± 0.52%)}	_~	_1.04s	_1.05s	_{p=0.441 n=6}
_{Total Time}	_{19.17s (± 0.26%)}	_{19.12s (± 0.24%)}	_~	_19.04s	_19.16s	_{p=0.171 n=6}
_{vscode - node (v20.5.1, x64)}
_{Memory used}	_{2,827,995k (± 0.00%)}	_{2,828,008k (± 0.00%)}	_~	_2,827,977k	_2,828,096k	_{p=0.689 n=6}
_{Parse Time}	_{10.73s (± 0.41%)}	_{10.73s (± 0.17%)}	_~	_10.71s	_10.76s	_{p=0.936 n=6}
_{Bind Time}	_{3.42s (± 0.47%)}	_{3.43s (± 0.40%)}	_~	_3.41s	_3.44s	_{p=0.514 n=6}
_{Check Time}	_{56.17s (± 0.15%)}	_{56.16s (± 0.24%)}	_~	_55.97s	_56.36s	_{p=0.872 n=6}
_{Emit Time}	_{16.20s (± 0.96%)}	_{16.27s (± 0.46%)}	_~	_16.17s	_16.36s	_{p=0.261 n=6}
_{Total Time}	_{86.53s (± 0.19%)}	_{86.59s (± 0.18%)}	_~	_86.34s	_86.77s	_{p=0.470 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-09T19:31:31Z

@weswigham Here are the results of running the top-repos suite comparing main and refs/pull/56857/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

acutmore · 2024-02-14T22:16:04Z

Just a note to say that this change seems to have regressed our (Bloomberg's) workaround for #38111.

I'll put a playground together and raise a concrete issue tomorrow 🙂

Emit declarations using alternative containing modules for types expo…

5ef3a8e

…rted using separate export statements

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

Andarist commented Dec 23, 2023

View reviewed changes

Andarist mentioned this pull request Dec 23, 2023

[vue-query]: Fresh Vue3 project + Tanstack - Typescript errors, This is likely not a portable. TanStack/query#6318

Closed

weswigham requested changes Jan 2, 2024

View reviewed changes

sandersn assigned weswigham Jan 4, 2024

typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 4, 2024

sandersn requested a review from sheetalkamat January 4, 2024 22:13

getWithAlternativeContainers for all containers

23553bb

Andarist force-pushed the fix/dts-emit-through-separate-export-statements branch from 4b7ce35 to 23553bb Compare January 6, 2024 08:58

weswigham approved these changes Jan 9, 2024

View reviewed changes

weswigham merged commit 4557e34 into microsoft:main Jan 9, 2024
19 checks passed

This was referenced Feb 15, 2024

[TS 5.4.0-beta] .d.ts emit adding an inline type import instead of utilising existing import #57415

Open

Bloomberg feedback for 5.4 Beta #57464

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit declarations using alternative containing modules for types exported using separate export statements #56857

Emit declarations using alternative containing modules for types exported using separate export statements #56857

Andarist commented Dec 23, 2023

Andarist Dec 23, 2023

fatcerberus Dec 23, 2023

Andarist Dec 23, 2023

weswigham Jan 2, 2024

Andarist Jan 6, 2024

weswigham Jan 2, 2024

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

acutmore commented Feb 14, 2024

Emit declarations using alternative containing modules for types exported using separate export statements #56857

Emit declarations using alternative containing modules for types exported using separate export statements #56857

Conversation

Andarist commented Dec 23, 2023

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

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

acutmore commented Feb 14, 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