Add TReturn/TNext to Iterable et al #58243

rbuckton · 2024-04-18T18:29:17Z

We avoided making this change in the past as it was very breaky, but at some point we need to address this discrepancy. Having incorrect types here is also causing problems with properly typing the Iterator Helpers proposal.

This also adds a new BuiltinIteratorReturn intrinsic type whose actual type is determined by the state of a new strictBuiltinIteratorReturn compiler option:

"strictBuiltinIteratorReturn": false - any (emulates current behavior for IterableIterator)
"strictBuiltinIteratorReturn": true - undefined

The strictBuiltinIteratorReturn is a strict option flag, meaning that it is enabled automatically when you set "strict": true in your tsconfig.json.

The new BuiltinIteratorReturn type is passed as the TReturn type argument for built-ins using IterableIterator or AsyncIterableIterator to enable stricter checks against the result of calling next() on the iterator.

Enabling strictBuiltinIteratorReturn results in a more accurate and type-safe return type for the next() methods of iterators produced by built-ins like Array, Set, Map, etc.:

// @strictBuiltinIteratorReturn: false
// NOTE: matches current behavior

const set = new Set(["a"]);
const result = set.keys().next();
//    ^?  result: IteratorResult<string, any>
const value = result.value;
//    ^?  value: any

if (!result.done) {
  const value = result.value;
  //    ^?  value: string
} else {
  const value = result.value;
  //    ^?  value: any
}

vs

// @strictBuiltinIteratorReturn: true

const set = new Set(["a"]);
const result = set.keys().next();
//    ^?  result: IteratorResult<string, undefined>
const value = result.value;
//    ^?  value: string | undefined

if (!result.done) {
  const value = result.value;
  //    ^?  value: string
} else {
  const value = result.value;
  //    ^?  value: undefined
}

Since this is a strict flag, there is a fair amount of existing code that will likely produce new compilation errors as a result of this change:

// @strict: true

function only<T>(set: Set<T>): T {
  if (set.size !== 1) throw new TypeError();
  return set.keys().next().value; // worked previously since result was `any`, but is now an error
}

This now fails as there is no correlation between set.size and the iterator produced by keys(), so the compiler is unaware that this constraint has been validated. If you are certain iterator will always yield at least one value, you can use a non-null assertion operator to strip off the | undefined:

// @strict: true

function only<T>(set: Set<T>): T {
  if (set.size !== 1) throw new TypeError();
  return set.keys().next().value!;
}

A follow-on PR to TypeScript-DOM-lib-generator can be found here: microsoft/TypeScript-DOM-lib-generator#1713

DefinitelyTyped breaks will be addressed by DefinitelyTyped/DefinitelyTyped#69632

fp-ts breaks will be addressed by gcanti/fp-ts#1949

webpack breaks will be addressed by webpack/webpack#18591

Fixes #33353
Fixes #52998
Fixes #43750
Closes #56517
Related #58222

rbuckton · 2024-04-19T01:15:09Z

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

There were interesting changes:

Changes are too big to display here, please check the log.

You can check the log here.

@jakebailey, @weswigham: Am I missing something? The bot says there were interesting changes but it links to a clean pipeline result.

rbuckton · 2024-07-18T19:27:21Z

I've added some tests for isolatedDeclarations to verify the expected outputs. This mostly effects inferred types though, so I'm not sure isolatedDeclarations would be impacted.

jakebailey · 2024-07-18T21:24:53Z

src/testRunner/compilerRunner.ts

-        "moduleDetection",
-        "moduleResolution",
+        // implicit variations from defined options
+        ...ts.optionDeclarations


Thank you for doing this, I was getting really tired of adding new flags and then wondering why I couldn't vary tests on them

jakebailey · 2024-07-18T21:45:52Z

src/compiler/checker.ts

+                if (type === intrinsicMarkerType && symbol.escapedName === "BuiltinIteratorReturn") {
+                    type = strictBuiltinIteratorReturn ? undefinedType : anyType;
+                }


Can this be done in the same way as IntrinsicTypeKind like IntrinsicTypeKind.NoInfer, rather than hardcoding the name in a couple of places?

No. IntrinsicTypeKind.NoInfer (and all of the other IntrinsicTypeKind entries) indicate a generic type mapping, e.g. type NoInfer<T> = intrinsic maps T to something else, which in that instance is a substitution type. Since BuiltinIteratorReturn is not generic it isn't picked up by getTypeAliasInstantiation, so never hits those code paths.

If anything, it would exist in its own map of non-generic intrinsic types, but since it would be the only entry there's no need for a map yet. When I started implementing this, I considered adding an arity to intrinsicTypeKinds, but it was far too complex to be worth it.

jakebailey · 2024-07-18T23:40:31Z

I believe this is what we decided on in the meeting and seems sound; I think that all that's left here is just to run perf on the final state (I don't think its been done in a bit, just the extended tests).

At least, once ADO is back online: https://status.dev.azure.com/

rbuckton · 2024-07-19T00:13:33Z

@typescript-bot perf test

typescript-bot · 2024-07-19T00:13:37Z

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

Command	Status	Results
`perf test`	❌ Error: `Error:

rbuckton · 2024-07-19T00:17:09Z

The perf shouldn't have changed since the only real changes since it was last run were swapping around what type to use when the flag is enabled, but I'll try it again when the AzDO outage is over

rbuckton · 2024-07-19T05:35:55Z

@typescript-bot perf test

typescript-bot · 2024-07-19T05:35:59Z

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

Command	Status	Results
`perf test`	✅ Started	👀 Results

typescript-bot · 2024-07-19T06:14:58Z

@rbuckton
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,447k (± 0.94%)}	_{192,835k (± 0.72%)}	_~	_192,226k	_195,677k	_{p=0.378 n=6}
_{Parse Time}	_{1.96s (± 1.27%)}	_{1.95s (± 0.75%)}	_~	_1.93s	_1.97s	_{p=0.222 n=6}
_{Bind Time}	_{1.06s (± 0.84%)}	_{1.06s (± 0.77%)}	_~	_1.05s	_1.07s	_{p=0.550 n=6}
_{Check Time}	_{13.97s (± 0.38%)}	_{13.95s (± 0.39%)}	_~	_13.89s	_14.01s	_{p=0.687 n=6}
_{Emit Time}	_{4.03s (± 0.73%)}	_{3.99s (± 0.97%)}	_~	_3.92s	_4.02s	_{p=0.076 n=6}
_{Total Time}	_{21.02s (± 0.20%)}	_{20.95s (± 0.32%)}	_{-0.08s (- 0.36%)}	_20.88s	_21.07s	_{p=0.045 n=6}
_{angular-1 - node (v18.15.0, x64)}
_Errors	₅	₇	_{🔻+2 (+40.00%)}	_~	_~	_{p=0.001 n=6}
_Symbols	_944,250	_945,484	_{+1,234 (+ 0.13%)}	_~	_~	_{p=0.001 n=6}
_Types	_407,076	_409,343	_{+2,267 (+ 0.56%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{1,218,683k (± 0.00%)}	_{1,220,806k (± 0.00%)}	_{+2,124k (+ 0.17%)}	_1,220,745k	_1,220,853k	_{p=0.005 n=6}
_{Parse Time}	_{8.02s (± 0.56%)}	_{7.90s (± 0.45%)}	_{-0.13s (- 1.56%)}	_7.84s	_7.93s	_{p=0.005 n=6}
_{Bind Time}	_{2.23s (± 0.34%)}	_{2.24s (± 0.77%)}	_~	_2.22s	_2.26s	_{p=0.404 n=6}
_{Check Time}	_{36.38s (± 0.63%)}	_{36.34s (± 0.45%)}	_~	_36.18s	_36.60s	_{p=0.872 n=6}
_{Emit Time}	_{17.88s (± 0.72%)}	_{17.94s (± 0.42%)}	_~	_17.84s	_18.05s	_{p=0.470 n=6}
_{Total Time}	_{64.52s (± 0.52%)}	_{64.42s (± 0.33%)}	_~	_64.20s	_64.77s	_{p=0.810 n=6}
_{mui-docs - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,276,156	_2,276,267	_{+111 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_948,845	_949,009	_{+164 (+ 0.02%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,207,580k (± 0.00%)}	_{2,207,979k (± 0.00%)}	_{+398k (+ 0.02%)}	_2,207,770k	_2,208,072k	_{p=0.005 n=6}
_{Parse Time}	_{9.67s (± 0.44%)}	_{9.67s (± 0.35%)}	_~	_9.61s	_9.70s	_{p=0.468 n=6}
_{Bind Time}	_{3.38s (± 0.74%)}	_{3.38s (± 1.29%)}	_~	_3.32s	_3.43s	_{p=0.935 n=6}
_{Check Time}	_{105.60s (± 0.57%)}	_{105.62s (± 0.41%)}	_~	_105.18s	_106.43s	_{p=1.000 n=6}
_{Emit Time}	_{0.20s (± 2.62%)}	_{0.20s (± 4.01%)}	_~	_0.19s	_0.21s	_{p=0.140 n=6}
_{Total Time}	_{118.85s (± 0.54%)}	_{118.87s (± 0.37%)}	_~	_118.50s	_119.74s	_{p=0.936 n=6}
_{self-build-src - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,227,332	_1,230,214	_{+2,882 (+ 0.23%)}	_~	_~	_{p=0.001 n=6}
_Types	_261,851	_265,676	_{+3,825 (+ 1.46%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,401,304k (± 6.04%)}	_{2,465,455k (± 7.46%)}	_{+64,151k (+ 2.67%)}	_2,345,723k	_2,703,780k	_{p=0.031 n=6}
_{Parse Time}	_{6.08s (± 0.92%)}	_{6.08s (± 0.60%)}	_~	_6.04s	_6.13s	_{p=0.630 n=6}
_{Bind Time}	_{2.26s (± 0.46%)}	_{2.26s (± 1.14%)}	_~	_2.22s	_2.28s	_{p=0.871 n=6}
_{Check Time}	_{40.54s (± 0.72%)}	_{40.62s (± 0.60%)}	_~	_40.29s	_40.91s	_{p=0.810 n=6}
_{Emit Time}	_{3.82s (± 1.38%)}	_{3.89s (± 1.49%)}	_~	_3.83s	_3.97s	_{p=0.092 n=6}
_{Total Time}	_{52.72s (± 0.60%)}	_{52.84s (± 0.45%)}	_~	_52.53s	_53.11s	_{p=0.748 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,227,332	_1,230,214	_{+2,882 (+ 0.23%)}	_~	_~	_{p=0.001 n=6}
_Types	_261,851	_265,676	_{+3,825 (+ 1.46%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,415,224k (± 0.03%)}	_{2,420,788k (± 0.03%)}	_{+5,564k (+ 0.23%)}	_2,420,129k	_2,421,854k	_{p=0.005 n=6}
_{Parse Time}	_{5.27s (± 1.34%)}	_{5.27s (± 0.95%)}	_~	_5.22s	_5.34s	_{p=0.688 n=6}
_{Bind Time}	_{1.69s (± 0.69%)}	_{1.70s (± 0.78%)}	_~	_1.68s	_1.71s	_{p=0.206 n=6}
_{Check Time}	_{34.95s (± 0.35%)}	_{35.15s (± 0.28%)}	_{+0.20s (+ 0.57%)}	_35.02s	_35.32s	_{p=0.013 n=6}
_{Emit Time}	_{3.37s (± 1.69%)}	_{3.34s (± 1.73%)}	_~	_3.29s	_3.45s	_{p=0.377 n=6}
_{Total Time}	_{45.28s (± 0.38%)}	_{45.46s (± 0.22%)}	_~	_45.27s	_45.57s	_{p=0.092 n=6}
_{self-compiler - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_258,516	_258,669	_{+153 (+ 0.06%)}	_~	_~	_{p=0.001 n=6}
_Types	_104,901	_105,403	_{+502 (+ 0.48%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{427,887k (± 0.02%)}	_{428,292k (± 0.02%)}	_{+404k (+ 0.09%)}	_428,174k	_428,457k	_{p=0.005 n=6}
_{Parse Time}	_{4.14s (± 0.43%)}	_{4.14s (± 0.37%)}	_~	_4.12s	_4.16s	_{p=1.000 n=6}
_{Bind Time}	_{1.62s (± 1.68%)}	_{1.64s (± 0.75%)}	_~	_1.62s	_1.65s	_{p=0.491 n=6}
_{Check Time}	_{22.18s (± 0.22%)}	_{22.23s (± 0.25%)}	_~	_22.17s	_22.31s	_{p=0.142 n=6}
_{Emit Time}	_{2.00s (± 1.03%)}	_{2.01s (± 0.75%)}	_~	_1.98s	_2.02s	_{p=0.371 n=6}
_{Total Time}	_{29.94s (± 0.25%)}	_{30.01s (± 0.17%)}	_~	_29.96s	_30.08s	_{p=0.145 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_Errors	₃₅	₃₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_224,565	_224,852	_{+287 (+ 0.13%)}	_~	_~	_{p=0.001 n=6}
_Types	_93,734	_94,040	_{+306 (+ 0.33%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{369,634k (± 0.03%)}	_{369,874k (± 0.04%)}	_{+240k (+ 0.06%)}	_369,726k	_370,159k	_{p=0.008 n=6}
_{Parse Time}	_{3.47s (± 0.43%)}	_{3.44s (± 0.48%)}	_{-0.03s (- 0.91%)}	_3.41s	_3.46s	_{p=0.015 n=6}
_{Bind Time}	_{1.93s (± 0.63%)}	_{1.93s (± 1.07%)}	_~	_1.91s	_1.96s	_{p=0.935 n=6}
_{Check Time}	_{19.33s (± 0.52%)}	_{19.25s (± 0.25%)}	_~	_19.18s	_19.31s	_{p=0.128 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{24.73s (± 0.42%)}	_{24.62s (± 0.20%)}	_~	_24.55s	_24.67s	_{p=0.108 n=6}
_{vscode - node (v18.15.0, x64)}
_Errors	₀	₄	_{🔻+4 (+ ∞%)}	_~	_~	_{p=0.001 n=6}
_Symbols	_2,973,349	_2,974,196	_{+847 (+ 0.03%)}	_~	_~	_{p=0.001 n=6}
_Types	_1,021,157	_1,023,116	_{+1,959 (+ 0.19%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{3,097,712k (± 0.00%)}	_{3,099,803k (± 0.00%)}	_{+2,091k (+ 0.07%)}	_3,099,760k	_3,099,855k	_{p=0.005 n=6}
_{Parse Time}	_{11.65s (± 0.24%)}	_{11.63s (± 0.51%)}	_~	_11.55s	_11.70s	_{p=0.688 n=6}
_{Bind Time}	_{3.56s (± 0.33%)}	_{3.59s (± 2.19%)}	_~	_3.55s	_3.75s	_{p=0.804 n=6}
_{Check Time}	_{68.75s (± 0.35%)}	_{68.67s (± 0.27%)}	_~	_68.39s	_68.86s	_{p=0.688 n=6}
_{Emit Time}	_{17.27s (± 0.78%)}	_{17.48s (± 0.66%)}	_{+0.20s (+ 1.17%)}	_17.27s	_17.58s	_{p=0.030 n=6}
_{Total Time}	_{101.25s (± 0.29%)}	_{101.36s (± 0.29%)}	_~	_100.96s	_101.69s	_{p=0.575 n=6}
_{webpack - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_267,370	_267,518	_{+148 (+ 0.06%)}	_~	_~	_{p=0.001 n=6}
_Types	_108,862	_109,024	_{+162 (+ 0.15%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{411,934k (± 0.01%)}	_{412,274k (± 0.01%)}	_{+340k (+ 0.08%)}	_412,209k	_412,344k	_{p=0.005 n=6}
_{Parse Time}	_{3.19s (± 0.57%)}	_{3.17s (± 0.33%)}	_~	_3.16s	_3.19s	_{p=0.249 n=6}
_{Bind Time}	_{1.41s (± 0.37%)}	_{1.41s (± 0.39%)}	_~	_1.40s	_1.41s	_{p=0.640 n=6}
_{Check Time}	_{14.37s (± 0.59%)}	_{14.29s (± 0.52%)}	_~	_14.18s	_14.41s	_{p=0.128 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{18.96s (± 0.53%)}	_{18.87s (± 0.43%)}	_~	_18.75s	_19.00s	_{p=0.128 n=6}
_{xstate-main - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_510,573	_510,625	_{+52 (+ 0.01%)}	_~	_~	_{p=0.001 n=6}
_Types	_161,621	_161,746	_{+125 (+ 0.08%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{448,287k (± 0.09%)}	_{448,438k (± 0.03%)}	_~	_448,274k	_448,642k	_{p=0.936 n=6}
_{Parse Time}	_{3.90s (± 0.56%)}	_{3.90s (± 0.45%)}	_~	_3.87s	_3.91s	_{p=1.000 n=6}
_{Bind Time}	_{1.45s (± 0.52%)}	_{1.44s (± 0.52%)}	_{-0.01s (- 0.92%)}	_1.43s	_1.45s	_{p=0.024 n=6}
_{Check Time}	_{21.18s (± 0.67%)}	_{21.23s (± 0.26%)}	_~	_21.16s	_21.32s	_{p=0.810 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{26.53s (± 0.61%)}	_{26.56s (± 0.25%)}	_~	_26.51s	_26.69s	_{p=0.810 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	₆

tsserver

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Compiler-UnionsTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{2,298ms (± 0.34%)}	_{2,302ms (± 0.34%)}	_~	_2,292ms	_2,310ms	_{p=0.575 n=6}
_{Req 2 - geterr}	_{5,171ms (± 0.43%)}	_{5,136ms (± 0.53%)}	_~	_5,101ms	_5,162ms	_{p=0.128 n=6}
_{Req 3 - references}	_{263ms (± 0.29%)}	_{262ms (± 1.10%)}	_~	_258ms	_264ms	_{p=0.672 n=6}
_{Req 4 - navto}	_{227ms (± 0.23%)}	_{227ms (± 0.56%)}	_~	_226ms	_229ms	_{p=0.931 n=6}
_{Req 5 - completionInfo count}	_1,357	_1,357	_~	_~	_~	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{87ms (± 6.25%)}	_{88ms (± 3.68%)}	_~	_84ms	_92ms	_{p=0.807 n=6}
_{CompilerTSServer - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{3,576ms (± 0.74%)}	_{3,593ms (± 0.55%)}	_~	_3,572ms	_3,619ms	_{p=0.261 n=6}
_{Req 2 - geterr}	_{5,674ms (± 0.68%)}	_{5,671ms (± 0.42%)}	_~	_5,641ms	_5,698ms	_{p=0.936 n=6}
_{Req 3 - references}	_{413ms (± 1.56%)}	_{410ms (± 0.40%)}	_~	_408ms	_412ms	_{p=0.809 n=6}
_{Req 4 - navto}	_{341ms (± 0.72%)}	_{340ms (± 0.25%)}	_~	_338ms	_340ms	_{p=0.115 n=6}
_{Req 5 - completionInfo count}	_1,519	_1,519	_~	_~	_~	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{122ms (± 5.06%)}	_{121ms (± 5.40%)}	_~	_108ms	_125ms	_{p=0.934 n=6}
_{xstate-main-1-tsserver - node (v18.15.0, x64)}
_{Req 1 - updateOpen}	_{5,178ms (± 0.35%)}	_{5,181ms (± 0.17%)}	_~	_5,172ms	_5,195ms	_{p=0.810 n=6}
_{Req 2 - geterr}	_{1,119ms (± 1.27%)}	_{1,110ms (± 1.01%)}	_~	_1,099ms	_1,131ms	_{p=0.128 n=6}
_{Req 3 - references}	_{88ms (± 4.22%)}	_{89ms (± 3.32%)}	_~	_86ms	_92ms	_{p=0.797 n=6}
_{Req 4 - navto}	_{452ms (± 0.18%)}	_{442ms (± 0.18%)}	_{-9ms (- 2.07%)}	_441ms	_443ms	_{p=0.004 n=6}
_{Req 5 - completionInfo count}	_3,417	_3,417	_~	_~	_~	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{814ms (± 0.53%)}	_{805ms (± 0.95%)}	_~	_794ms	_816ms	_{p=0.053 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

CompilerTSServer - node (v18.15.0, x64)
Compiler-UnionsTSServer - node (v18.15.0, x64)
xstate-main-1-tsserver - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

startup

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{tsc-startup - node (v18.15.0, x64)}
_{Execution time}	_{228.66ms (± 0.16%)}	_{228.52ms (± 0.14%)}	_{-0.14ms (- 0.06%)}	_226.93ms	_230.79ms	_{p=0.000 n=600}
_{tsserver-startup - node (v18.15.0, x64)}
_{Execution time}	_{347.82ms (± 0.31%)}	_{348.05ms (± 0.31%)}	_{+0.23ms (+ 0.07%)}	_340.17ms	_356.16ms	_{p=0.000 n=600}
_{tsserverlibrary-startup - node (v18.15.0, x64)}
_{Execution time}	_{227.77ms (± 0.13%)}	_{227.69ms (± 0.15%)}	_{-0.08ms (- 0.04%)}	_226.18ms	_232.43ms	_{p=0.000 n=600}
_{typescript-startup - node (v18.15.0, x64)}
_{Execution time}	_{226.19ms (± 0.15%)}	_{226.17ms (± 0.16%)}	_~	_224.42ms	_230.49ms	_{p=0.358 n=600}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

tsc-startup - node (v18.15.0, x64)
tsserver-startup - node (v18.15.0, x64)
tsserverlibrary-startup - node (v18.15.0, x64)
typescript-startup - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

rbuckton · 2024-07-19T16:03:03Z

@jakebailey: It looks like the perf run was nominal.

src/compiler/checker.ts

sandersn · 2024-07-19T15:34:00Z

src/compiler/checker.ts

+                        if (type.target.typeParameters) {
+                            typeParameterCount = Math.min(type.target.typeParameters.length, typeArguments.length);
+
+                            // Maybe we should do this for more types, but for now we only elide type arguments that are


what other kinds of types would benefit from this?

Potentially any type since this simplifies the emit output to avoid repeating type arguments for type parameters with defaults, but rather than take that on in this PR I focused on this subset to improve back compat.

sandersn · 2024-07-19T15:38:55Z

src/compiler/checker.ts

+                                    type.node.typeArguments.length < typeParameterCount
+                                ) {
+                                    while (typeParameterCount > 0) {
+                                        const typeArgument = typeArguments[typeParameterCount - 1];


an incidental question, but: what is a case where typeArguments.length > 0 but also !type.node.typeArguments is true?

type argument inference

sandersn · 2024-07-19T15:40:40Z

src/compiler/checker.ts

+                            // Maybe we should do this for more types, but for now we only elide type arguments that are
+                            // identical to their associated type parameters' defaults for `Iterable`, `IterableIterator`,
+                            // `AsyncIterable`, and `AsyncIterableIterator` to provide backwards-compatible .d.ts emit due
+                            // to each now having three type parameters instead of only one.


overall this seems like an odd way to support this backward compatibility, but I don't understand the problem space well enough to suggest anything else

Realistically, this is all about what the type "looks" like, right? Or are we really needing to depend on this emit still emitting the "old thing" to continue to work right for downstream users of d.ts files?

Consider an application that is currently built using TS 5.5 that depends on a library that is also being built using TS 5.5. The library chooses to upgrade its version of TypeScript to 5.6, and the new emit produces Iterable<T, any, any> where it previously produced Iterable<T>. If the library makes no other code changes and publishes their new build, the application cannot take the update without also updating TypeScript because the expected arity of Iterable has changed.
The library can't just ship a forward declaration of interface Iterable<T, TReturn = any, TNext = any> {}, because our checker performs arity checks on built-ins, and the application might not be running with skipLibCheck, so one way or another they would have errors they cannot silence without possibly giving up the dependency verification that skipLibChecks: false offers.

sandersn · 2024-07-19T16:28:00Z

src/harness/collectionsImpl.ts

@@ -132,6 +132,7 @@ export class SortedMap<K, V> {
                this._copyOnWrite = false;
            }
        }
+        return undefined;


will all generators need to return undefined now? do you have an estimate of how much code that breaks?

from reading type baselines, it looks like their return type changes to any instead of undefined

This is specifically because this is an implementation of the native Map<K, V> interface, which is now specified to return IterableIterator<T, BuiltinIteratorReturn> (which is IterableIterator<T, undefined> under the new flag).

jakebailey · 2024-07-19T16:40:50Z

I tested this on the most iterator-y codebase I know (effect) and it only caused one break (positively), so that's good.

rbuckton · 2024-07-19T16:55:41Z

I tested this on the most iterator-y codebase I know (effect) and it only caused one break (positively), so that's good.

In all of the PRs I've created so far for top libraries, it's usually only been one or two cases where they were (possibly unknowingly) depending on the result of set.next().value being typed as any by default.

sandersn

I still have a couple of background questions. I think this PR is worth trying in the beta to see if it's too breaky. But all the first-order breaks look good, and the higher-order ones are, I bet, rare. (like, just fpt-ts :)

Add TReturn/TNext to Iterable et al

ef2e1d9

typescript-bot assigned rbuckton Apr 18, 2024

typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 18, 2024

This comment was marked as outdated.

Sign in to view

rbuckton added 3 commits April 18, 2024 16:51

Fix assignability checks, update baselines

1a64ac9

Fix lint

7aa2281

Revert incorrect lint fix

be600a8

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into iterator-default-return

afbfa69

This comment was marked as outdated.

Sign in to view

Fix failing tests

8182bfc

This comment was marked as outdated.

Sign in to view

rbuckton requested a review from weswigham July 18, 2024 20:51

jakebailey reviewed Jul 18, 2024

View reviewed changes

Fix typo in comment

b8f04f6

sandersn reviewed Jul 19, 2024

View reviewed changes

Move initialization of 'typeParameterCount'

1a3669a

sandersn approved these changes Jul 19, 2024

View reviewed changes

rbuckton merged commit 6f530cc into main Jul 19, 2024
30 checks passed

rbuckton deleted the iterator-default-return branch July 19, 2024 17:14

This was referenced Jul 19, 2024

Use tuple name inference for Iterator.next et al #59360

Merged

Add BuiltInIterableIterator for built-in iterables with undefined return values #56517

Closed

Introduce ES2024 target and fix some types #58573

Open

NotWoods mentioned this pull request Jul 26, 2024

Flag to type Promise.catch variables as unknown #45602

Open

5 tasks

uhyo mentioned this pull request Jul 27, 2024

TS 5.6 Beta BuiltinIteratorReturn: can this be the default type argument of BuiltinIterator's TReturn? #59444

Closed

1 task

rbuckton mentioned this pull request Aug 1, 2024

Disambiguate BuiltinIterator/BuiltinIteratorReturn #59506

Merged

mrginglymus mentioned this pull request Aug 19, 2024

TS 5.6.0-beta compilation error in a project consuming lru-cache isaacs/node-lru-cache#348

Closed

jakebailey mentioned this pull request Aug 20, 2024

Drop unnecessary type arguments in the isolated declarations quick fix #59665

Merged

andrewbranch mentioned this pull request Sep 6, 2024

Type errors in TypeScript 5.6 due to lib changes (both in source and declaration files) isaacs/node-lru-cache#352

Closed

romaricpascal mentioned this pull request Sep 16, 2024

Bump the lint group with 2 updates alphagov/govuk-frontend#5314

Open

Add TReturn/TNext to Iterable et al #58243

Add TReturn/TNext to Iterable et al #58243

Conversation

rbuckton commented Apr 18, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

rbuckton commented Apr 19, 2024 • edited Loading

rbuckton commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

jakebailey commented Jul 18, 2024

rbuckton commented Jul 19, 2024

typescript-bot commented Jul 19, 2024 • edited Loading

rbuckton commented Jul 19, 2024

rbuckton commented Jul 19, 2024

typescript-bot commented Jul 19, 2024 • edited Loading

typescript-bot commented Jul 19, 2024

tsc

tsserver

startup

rbuckton commented Jul 19, 2024

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

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

jakebailey commented Jul 19, 2024

rbuckton commented Jul 19, 2024

sandersn left a comment

Choose a reason for hiding this comment

rbuckton commented Apr 18, 2024 •

edited

Loading

rbuckton commented Apr 19, 2024 •

edited

Loading

rbuckton Jul 18, 2024 •

edited

Loading

typescript-bot commented Jul 19, 2024 •

edited

Loading

typescript-bot commented Jul 19, 2024 •

edited

Loading