Do not perserve references in declaration emit, unless preserve=true #57681

jakebailey · 2024-03-07T19:14:26Z

At the moment, declaration emit does a funky mixture of preserving some references, eliding unused references, adding of referenced files for ambients (like node's builtins), but also sometimes imports (react), and so on. For external implementations of isolatedDeclarations (and honestly, the human brain), this is either difficult to reason about or totally impossible to replicate.

This PR greatly changes the way we handle references. References are no longer preserved in the output from the input, nor are they ever added based on any analysis. If a reference is intended to be preserved, one must write preserve="true" to have it copied to the output file. This gives a "what you see is what you get" format, not unlike what verbatimModuleSyntax does for imported values.

This is technically a break, as packages which re-export from something like @types/node will no longer have implicitly added reference directives for those types, however this change is in line with how we handle lib.d.ts; it's assumed that the user will have actually included the environment's types into their project. Testing in prep for this change (#57569, #57656) showed that most instances of added references fit into this bucket and should continue to work (and why we are shipping this without a new flag).

This change seems to improve performance a bit (1-4% in benchmarks with dts emit enabled), likely due to the reduced bookkeeping.

The only non-obvious thing that remains is the rewriting of file references, which includes the changing of import extensions (funny) and the shifting of paths to be relative to the output paths. This specific analysis is more reasonable to implement externally as it's just a function of file paths and compiler options.

Fixes #57439 (though does the opposite by default)

It also fixes these issues by nature of us no longer synthesizing new references, or just keeping the references as written:

Fixes #15488
Fixes #48143
Fixes #52110
Fixes #56590
Fixes #57482

Then in terms of design, the direction is now "we don't generate references out of thin air anymore", in which case this also closes:

Closes #28195

Related: #56571

typescript-bot · 2024-03-07T19:14:43Z

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 · 2024-03-07T19:16:09Z

A significant number of our tests break because they don't include the new references; is this an indicator that the default should actually be preserve=true? Or that our tests are "weird code heavy"?

sheetalkamat · 2024-03-07T22:03:30Z

@jakebailey our tests are written such that just including d.ts files should not result in error. You would need to look at what all changes need to happen in the config for building those declaration file.

jakebailey · 2024-03-07T22:16:32Z

The errors are declaration errors in the emit, since references that were needed were not preserved by default.

src/compiler/transformers/declarations.ts

src/compiler/parser.ts

tests/baselines/reference/nodeModulesTripleSlashReferenceModeDeclarationEmit3(module=node16).js

typescript-bot · 2024-03-20T17:09:25Z

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`perf test this faster`	✅ Started	👀 Results

typescript-bot · 2024-03-20T17:57:38Z

@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
_{Angular - node (v18.15.0, x64)}
_{Memory used}	_{295,741k (± 0.01%)}	_{295,605k (± 0.01%)}	_{-136k (- 0.05%)}	_295,561k	_295,628k	_{p=0.005 n=6}
_{Parse Time}	_{2.66s (± 0.44%)}	_{2.66s (± 0.48%)}	_~	_2.64s	_2.67s	_{p=1.000 n=6}
_{Bind Time}	_{0.83s (± 0.49%)}	_{0.83s (± 0.49%)}	_~	_0.83s	_0.84s	_{p=1.000 n=6}
_{Check Time}	_{8.19s (± 0.26%)}	_{8.23s (± 0.39%)}	_~	_8.18s	_8.26s	_{p=0.090 n=6}
_{Emit Time}	_{7.12s (± 0.55%)}	_{7.04s (± 0.31%)}	_{-0.09s (- 1.22%)}	_7.01s	_7.06s	_{p=0.005 n=6}
_{Total Time}	_{18.81s (± 0.16%)}	_{18.75s (± 0.18%)}	_{-0.06s (- 0.31%)}	_18.71s	_18.80s	_{p=0.025 n=6}
_{Compiler-Unions - node (v18.15.0, x64)}
_{Memory used}	_{193,205k (± 0.90%)}	_{192,528k (± 0.75%)}	_~	_191,853k	_195,479k	_{p=0.230 n=6}
_{Parse Time}	_{1.36s (± 0.46%)}	_{1.36s (± 1.23%)}	_~	_1.33s	_1.38s	_{p=0.720 n=6}
_{Bind Time}	_{0.72s (± 0.00%)}	_{0.72s (± 0.00%)}	_~	_0.72s	_0.72s	_{p=1.000 n=6}
_{Check Time}	_{9.51s (± 0.81%)}	_{9.51s (± 0.75%)}	_~	_9.42s	_9.62s	_{p=1.000 n=6}
_{Emit Time}	_{2.65s (± 0.44%)}	_{2.63s (± 0.32%)}	_{-0.02s (- 0.88%)}	_2.61s	_2.63s	_{p=0.010 n=6}
_{Total Time}	_{14.24s (± 0.64%)}	_{14.22s (± 0.57%)}	_~	_14.12s	_14.34s	_{p=0.688 n=6}
_{Monaco - node (v18.15.0, x64)}
_{Memory used}	_{347,384k (± 0.01%)}	_{347,378k (± 0.01%)}	_~	_347,346k	_347,416k	_{p=0.689 n=6}
_{Parse Time}	_{2.47s (± 0.44%)}	_{2.49s (± 0.55%)}	_~	_2.47s	_2.51s	_{p=0.054 n=6}
_{Bind Time}	_{0.93s (± 0.56%)}	_{0.93s (± 0.00%)}	_~	_0.93s	_0.93s	_{p=0.174 n=6}
_{Check Time}	_{7.02s (± 0.37%)}	_{7.04s (± 0.24%)}	_~	_7.01s	_7.06s	_{p=0.291 n=6}
_{Emit Time}	_{4.06s (± 0.40%)}	_{4.07s (± 0.48%)}	_~	_4.04s	_4.10s	_{p=1.000 n=6}
_{Total Time}	_{14.49s (± 0.23%)}	_{14.52s (± 0.24%)}	_~	_14.47s	_14.57s	_{p=0.169 n=6}
_{TFS - node (v18.15.0, x64)}
_{Memory used}	_{302,753k (± 0.01%)}	_{302,711k (± 0.00%)}	_{-42k (- 0.01%)}	_302,701k	_302,725k	_{p=0.005 n=6}
_{Parse Time}	_{2.00s (± 0.93%)}	_{2.01s (± 0.54%)}	_~	_2.00s	_2.03s	_{p=0.411 n=6}
_{Bind Time}	_{1.01s (± 1.08%)}	_{1.00s (± 0.41%)}	_~	_1.00s	_1.01s	_{p=0.090 n=6}
_{Check Time}	_{6.30s (± 0.39%)}	_{6.32s (± 0.48%)}	_~	_6.29s	_6.36s	_{p=0.332 n=6}
_{Emit Time}	_{3.60s (± 0.55%)}	_{3.62s (± 0.48%)}	_~	_3.60s	_3.64s	_{p=0.075 n=6}
_{Total Time}	_{12.92s (± 0.15%)}	_{12.96s (± 0.25%)}	_{+0.04s (+ 0.31%)}	_12.90s	_12.99s	_{p=0.036 n=6}
_{material-ui - node (v18.15.0, x64)}
_{Memory used}	_{509,926k (± 0.00%)}	_{509,880k (± 0.00%)}	_{-46k (- 0.01%)}	_509,858k	_509,902k	_{p=0.005 n=6}
_{Parse Time}	_{2.66s (± 0.41%)}	_{2.65s (± 0.78%)}	_~	_2.63s	_2.68s	_{p=0.801 n=6}
_{Bind Time}	_{0.99s (± 0.83%)}	_{0.99s (± 0.76%)}	_~	_0.98s	_1.00s	_{p=0.729 n=6}
_{Check Time}	_{17.23s (± 0.16%)}	_{17.23s (± 0.25%)}	_~	_17.16s	_17.28s	_{p=1.000 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{20.88s (± 0.13%)}	_{20.87s (± 0.19%)}	_~	_20.81s	_20.92s	_{p=0.871 n=6}
_{mui-docs - node (v18.15.0, x64)}
_{Memory used}	_{1,719,308k (± 0.00%)}	_{1,719,280k (± 0.00%)}	_~	_1,719,241k	_1,719,331k	_{p=0.173 n=6}
_{Parse Time}	_{6.52s (± 0.38%)}	_{6.54s (± 0.47%)}	_~	_6.49s	_6.58s	_{p=0.226 n=6}
_{Bind Time}	_{2.36s (± 0.35%)}	_{2.36s (± 0.35%)}	_~	_2.35s	_2.37s	_{p=1.000 n=6}
_{Check Time}	_{56.42s (± 0.54%)}	_{56.20s (± 0.33%)}	_~	_55.96s	_56.40s	_{p=0.128 n=6}
_{Emit Time}	_{0.13s (± 3.87%)}	_{0.13s (± 3.10%)}	_~	_0.13s	_0.14s	_{p=0.595 n=6}
_{Total Time}	_{65.43s (± 0.49%)}	_{65.23s (± 0.27%)}	_~	_65.00s	_65.39s	_{p=0.127 n=6}
_{self-build-src - node (v18.15.0, x64)}
_{Memory used}	_{2,396,016k (± 0.04%)}	_{2,391,983k (± 0.04%)}	_{-4,033k (- 0.17%)}	_2,391,032k	_2,393,513k	_{p=0.005 n=6}
_{Parse Time}	_{4.99s (± 1.17%)}	_{5.00s (± 0.76%)}	_~	_4.95s	_5.05s	_{p=1.000 n=6}
_{Bind Time}	_{1.89s (± 1.19%)}	_{1.90s (± 0.62%)}	_~	_1.88s	_1.91s	_{p=0.514 n=6}
_{Check Time}	_{33.51s (± 0.28%)}	_{33.63s (± 0.44%)}	_~	_33.48s	_33.80s	_{p=0.092 n=6}
_{Emit Time}	_{2.73s (± 1.30%)}	_{2.62s (± 0.88%)}	_{🟩-0.11s (- 3.91%)}	_2.59s	_2.66s	_{p=0.005 n=6}
_{Total Time}	_{43.14s (± 0.36%)}	_{43.16s (± 0.37%)}	_~	_42.99s	_43.37s	_{p=0.936 n=6}
_{self-compiler - node (v18.15.0, x64)}
_{Memory used}	_{416,105k (± 0.01%)}	_{415,087k (± 0.01%)}	_{-1,019k (- 0.24%)}	_415,037k	_415,123k	_{p=0.005 n=6}
_{Parse Time}	_{2.85s (± 1.01%)}	_{2.81s (± 1.57%)}	_~	_2.76s	_2.88s	_{p=0.196 n=6}
_{Bind Time}	_{1.07s (± 0.48%)}	_{1.07s (± 0.59%)}	_~	_1.06s	_1.08s	_{p=0.386 n=6}
_{Check Time}	_{15.30s (± 0.48%)}	_{15.28s (± 0.21%)}	_~	_15.24s	_15.33s	_{p=0.808 n=6}
_{Emit Time}	_{1.15s (± 0.89%)}	_{1.11s (± 1.23%)}	_{🟩-0.04s (- 3.47%)}	_1.10s	_1.13s	_{p=0.005 n=6}
_{Total Time}	_{20.36s (± 0.32%)}	_{20.28s (± 0.33%)}	_~	_20.19s	_20.34s	_{p=0.065 n=6}
_{vscode - node (v18.15.0, x64)}
_{Memory used}	_{2,887,474k (± 0.00%)}	_{2,887,473k (± 0.00%)}	_~	_2,887,365k	_2,887,571k	_{p=0.575 n=6}
_{Parse Time}	_{10.83s (± 0.40%)}	_{10.84s (± 0.30%)}	_~	_10.78s	_10.87s	_{p=0.686 n=6}
_{Bind Time}	_{3.46s (± 0.38%)}	_{3.46s (± 0.47%)}	_~	_3.43s	_3.48s	_{p=0.805 n=6}
_{Check Time}	_{61.51s (± 0.49%)}	_{61.50s (± 0.44%)}	_~	_61.26s	_61.99s	_{p=0.873 n=6}
_{Emit Time}	_{16.98s (± 7.80%)}	_{16.97s (± 8.48%)}	_~	_16.21s	_19.90s	_{p=0.630 n=6}
_{Total Time}	_{92.78s (± 1.70%)}	_{92.75s (± 1.84%)}	_~	_91.72s	_96.21s	_{p=0.810 n=6}
_{webpack - node (v18.15.0, x64)}
_{Memory used}	_{408,073k (± 0.02%)}	_{408,060k (± 0.02%)}	_~	_407,987k	_408,175k	_{p=0.936 n=6}
_{Parse Time}	_{3.21s (± 0.65%)}	_{3.24s (± 0.13%)}	_{+0.03s (+ 0.88%)}	_3.23s	_3.24s	_{p=0.006 n=6}
_{Bind Time}	_{1.38s (± 0.30%)}	_{1.38s (± 0.37%)}	_~	_1.37s	_1.38s	_{p=0.114 n=6}
_{Check Time}	_{14.29s (± 0.31%)}	_{14.31s (± 0.23%)}	_~	_14.28s	_14.35s	_{p=0.685 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{18.88s (± 0.31%)}	_{18.92s (± 0.20%)}	_~	_18.88s	_18.97s	_{p=0.470 n=6}
_{xstate - node (v18.15.0, x64)}
_{Memory used}	_{513,081k (± 0.01%)}	_{513,034k (± 0.01%)}	_~	_512,907k	_513,111k	_{p=0.423 n=6}
_{Parse Time}	_{3.28s (± 0.17%)}	_{3.28s (± 0.12%)}	_~	_3.28s	_3.29s	_{p=0.054 n=6}
_{Bind Time}	_{1.54s (± 0.26%)}	_{1.54s (± 0.33%)}	_~	_1.54s	_1.55s	_{p=0.595 n=6}
_{Check Time}	_{2.85s (± 0.36%)}	_{2.86s (± 0.82%)}	_~	_2.84s	_2.90s	_{p=0.220 n=6}
_{Emit Time}	_{0.07s (± 0.00%)}	_{0.07s (±11.12%)}	_~	_0.07s	_0.09s	_{p=0.405 n=6}
_{Total Time}	_{7.74s (± 0.15%)}	_{7.76s (± 0.23%)}	_~	_7.74s	_7.79s	_{p=0.089 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)
mui-docs - node (v18.15.0, x64)
self-build-src - node (v18.15.0, x64)
self-compiler - node (v18.15.0, x64)
vscode - node (v18.15.0, x64)
webpack - node (v18.15.0, x64)
xstate - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

jakebailey · 2024-03-20T18:09:49Z

Nice, now nearly 4% faster emit for the self benchmarks.

jakebailey · 2024-03-20T19:27:23Z

@typescript-bot pack this

typescript-bot · 2024-03-20T19:27:26Z

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
`pack this`	✅ Started	✅ Results

typescript-bot · 2024-03-20T19:32:23Z

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/160635/artifacts?artifactName=tgz&fileId=F9E2BFE8B70C68AFEF79932BEEF547DC885DD9935ECCD0249B1D1A4B70EC193402&fileName=/typescript-5.5.0-insiders.20240320.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/pr-build@5.5.0-pr-57681-18".;

jakebailey · 2024-03-20T19:47:16Z

Tested jupyterlab out; adding preserve=true fixes all problems. All of the other errors seem like oddities with how we attempt to build the project without actually invoking their build system.

See microsoft/TypeScript#57681

Do not perserve references in declaration emit, unless preserve=true

23ae8ae

typescript-bot assigned jakebailey Mar 7, 2024

typescript-bot added Author: Team For Backlog Bug PRs that fix a backlog bug labels Mar 7, 2024

typo

21111ae

jakebailey commented Mar 8, 2024

View reviewed changes

src/compiler/transformers/declarations.ts Outdated Show resolved Hide resolved

dragomirtitian mentioned this pull request Mar 8, 2024

[ID-Prep] PR2 - Preserve all type triple slash references #57440

Closed

DanielRosenwasser mentioned this pull request Mar 12, 2024

Design Meeting Notes, 3/12/2024 #57756

Closed

robpalme mentioned this pull request Mar 13, 2024

--isolatedDeclarations for standalone DTS emit #47947

Closed

5 tasks

jakebailey added 2 commits March 14, 2024 12:32

Merge branch 'main' into new-references-behavior

8c5ffa2

Update some tests with preserve

ac6d2b8

jakebailey mentioned this pull request Mar 14, 2024

Require exactly 'true' for truthy reference directive prop #57782

Merged

more test

0d05d6a

jakebailey commented Mar 14, 2024

View reviewed changes

src/compiler/parser.ts Outdated Show resolved Hide resolved

jakebailey added 5 commits March 15, 2024 14:38

Merge branch 'main' into new-references-behavior

bf31c91

Merge branch 'main' into new-references-behavior

07fed7b

match main

32dce6f

Merge branch 'main' into new-references-behavior

08d8192

baseline, lol

23f669f

sheetalkamat mentioned this pull request Mar 15, 2024

Make builder not depend on information from dts emit about really needed modules #57800

Merged

Merge branch 'main' into new-references-behavior

e3563c0

jakebailey commented Mar 18, 2024

View reviewed changes

tests/baselines/reference/nodeModulesTripleSlashReferenceModeDeclarationEmit3(module=node16).js Outdated Show resolved Hide resolved

sheetalkamat requested changes Mar 18, 2024

View reviewed changes

tests/baselines/reference/nodeModulesTripleSlashReferenceModeDeclarationEmit3(module=node16).js Outdated Show resolved Hide resolved

jakebailey added 3 commits March 18, 2024 15:36

Don't incorrectly dedupe refs

7e9d56b

more simplification

cf32724

more cleanup, so long moduleSpecifieirs

2259615

jakebailey added 2 commits March 20, 2024 11:19

Reduce some duplication

067c2e1

Explicitly reset on reuse

a08aa76

jakebailey requested a review from sheetalkamat March 20, 2024 19:05

sheetalkamat approved these changes Mar 20, 2024

View reviewed changes

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Mar 20, 2024

jakebailey merged commit 7d50455 into microsoft:main Mar 20, 2024
25 checks passed

jakebailey deleted the new-references-behavior branch March 20, 2024 22:06

jakebailey mentioned this pull request Mar 20, 2024

5.3.2 adds broken "reference" comment #56571

Closed

NullVoxPopuli mentioned this pull request Mar 20, 2024

Is the TS /// <reference issue fixed upstream? NullVoxPopuli/fix-bad-declaration-output#15

Open

jakebailey mentioned this pull request Mar 20, 2024

Preserve resolution-mode as-written too #57867

Merged

sheetalkamat mentioned this pull request Mar 21, 2024

Auto type reference directives do not lowercase the typing name #26948

Closed

andrewbranch mentioned this pull request Mar 22, 2024

Module resolution: resolution-mode="require" emitted too eagerly #56592

Closed

timocov added a commit to timocov/dts-bundle-generator that referenced this pull request Mar 24, 2024

Fixed tests for latest (next) typescript version

27b62bf

See microsoft/TypeScript#57681

RyanCavanaugh mentioned this pull request Apr 19, 2024

[NewErrors] 5.5.0-dev.20240414 vs 5.4.5 #58189

Open

robpalme mentioned this pull request Apr 26, 2024

Undocumented breaking change in 5.5.0-beta for reference directives #58324

Closed

sheetalkamat mentioned this pull request May 13, 2024

program.getResolvedTypeReferenceDirectives() not keyed by containing file/directory, leading to invalid declaration emit #56836

Closed

weswigham mentioned this pull request May 17, 2024

Module/interface augmentation doesn't preserve import for declaration emit #56528

Open

ijlee2 mentioned this pull request Jun 14, 2024

Provide workarounds for both TS (bad peer-provided types handling) and Glint (mishandling of extensions) issues embroider-build/addon-blueprint#292

Open

jakebailey mentioned this pull request Jun 21, 2024

/// <reference lib is stripped out of d.ts file #58956

Closed

justinfagnani mentioned this pull request Jun 24, 2024

Required triple-slash types reference no longer generated in TS 5.5 #58985

Closed

devjiwonchoi mentioned this pull request Jul 3, 2024

Upgrade TypeScript to v5.5.2 vercel/next.js#67113

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not perserve references in declaration emit, unless preserve=true #57681

Do not perserve references in declaration emit, unless preserve=true #57681

jakebailey commented Mar 7, 2024 •

edited

Loading

typescript-bot commented Mar 7, 2024

jakebailey commented Mar 7, 2024

sheetalkamat commented Mar 7, 2024

jakebailey commented Mar 7, 2024

typescript-bot commented Mar 20, 2024 •

edited

Loading

typescript-bot commented Mar 20, 2024

tsc

jakebailey commented Mar 20, 2024

jakebailey commented Mar 20, 2024

typescript-bot commented Mar 20, 2024 •

edited

Loading

typescript-bot commented Mar 20, 2024 •

edited

Loading

jakebailey commented Mar 20, 2024

Do not perserve references in declaration emit, unless preserve=true #57681

Do not perserve references in declaration emit, unless preserve=true #57681

Conversation

jakebailey commented Mar 7, 2024 • edited Loading

typescript-bot commented Mar 7, 2024

jakebailey commented Mar 7, 2024

sheetalkamat commented Mar 7, 2024

jakebailey commented Mar 7, 2024

typescript-bot commented Mar 20, 2024 • edited Loading

typescript-bot commented Mar 20, 2024

tsc

jakebailey commented Mar 20, 2024

jakebailey commented Mar 20, 2024

typescript-bot commented Mar 20, 2024 • edited Loading

typescript-bot commented Mar 20, 2024 • edited Loading

jakebailey commented Mar 20, 2024

jakebailey commented Mar 7, 2024 •

edited

Loading

typescript-bot commented Mar 20, 2024 •

edited

Loading

typescript-bot commented Mar 20, 2024 •

edited

Loading

typescript-bot commented Mar 20, 2024 •

edited

Loading