Do not infer yield* type from contextual TReturn
#58621
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
typescript-bot
added
Author: Team
For Milestone Bug
|
@typescript-bot perf test |
|
Starting jobs; this comment will be updated as builds start and complete.
|
|
@rbuckton Here are the results of running the user tests comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
Hey @rbuckton, the results of running the DT tests are ready. Everything looks the same! |
|
@rbuckton Here are the results of running the user tests comparing Everything looks good! |
|
@rbuckton Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rbuckton
changed the title
yield* type from contextual TReturn
|
@rbuckton Here are the results of running the top 200 repos comparing Something interesting changed - please have a look. DetailsServer exited prematurely with code unknown and signal SIGABRTAffected reposcalcom/cal.comRaw error text:RepoResults7/calcom.cal.com.rawError.txt in the artifact folder Replay commands: RepoResults7/calcom.cal.com.replay.txt in the artifact folder
Last few requests{"seq":89,"type":"request","command":"navto","arguments":{"searchValue":"a1R","maxResultCount":256}}
{"seq":90,"type":"request","command":"updateOpen","arguments":{"changedFiles":[{"fileName":"@PROJECT_ROOT@/packages/trpc/index.ts","textChanges":[{"newText":" //comment","start":{"line":1,"offset":26},"end":{"line":1,"offset":26}}]}],"closedFiles":[],"openFiles":[]}}
{"seq":91,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/packages/types/Credential.d.ts"],"openFiles":[]}}
{"seq":92,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/packages/prisma/seed-utils.ts","projectRootPath":"@PROJECT_ROOT@"}]}}
Repro steps#!/bin/bash
git clone https://github.com/calcom/cal.com --recurse-submodules
git -C "./cal.com" reset --hard 26bd02bd9409123f50f3501471738da66554f558
yarn --cwd "/mnt/ts_downloads/base/cal.com" install --no-immutable --mode=skip-build
downloadUrl=$(curl -s "https://typescript.visualstudio.com/TypeScript/_apis/build/builds/161860/artifacts?artifactName=RepoResults7&api-version=7.0" | jq -r ".resource.downloadUrl")
wget -O RepoResults7.zip "$downloadUrl"
unzip -p RepoResults7.zip RepoResults7/calcom.cal.com.replay.txt > calcom.cal.com.replay.txt
npm install --no-save @typescript/server-replayTo run the repro: # `npx tsreplay --help` to learn about helpful switches for debugging, logging, etc.
npx tsreplay ./cal.com ./calcom.cal.com.replay.txt <PATH_TO_tsserver.js>Server exited prematurely with code unknown and signal SIGABRTAffected reposelastic/kibanaRaw error text:RepoResults14/elastic.kibana.rawError.txt in the artifact folder Replay commands: RepoResults14/elastic.kibana.replay.txt in the artifact folder
Last few requests{"seq":712,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/x-pack/plugins/security_solution_serverless/public/types.ts","line":19,"offset":29,"includeExternalModuleExports":false,"triggerKind":1}}
{"seq":713,"type":"request","command":"references","arguments":{"file":"@PROJECT_ROOT@/x-pack/plugins/security_solution_serverless/public/types.ts","line":22,"offset":29}}
{"seq":714,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/x-pack/test/api_integration/services/slo.ts"],"openFiles":[]}}
{"seq":715,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/x-pack/plugins/security/server/plugin.test.ts","projectRootPath":"@PROJECT_ROOT@"}]}}
Repro steps#!/bin/bash
git clone https://github.com/elastic/kibana --recurse-submodules
git -C "./kibana" reset --hard 42afaa38a2bcd0fbbc331f3d391be81bbc1578e0
downloadUrl=$(curl -s "https://typescript.visualstudio.com/TypeScript/_apis/build/builds/161860/artifacts?artifactName=RepoResults14&api-version=7.0" | jq -r ".resource.downloadUrl")
wget -O RepoResults14.zip "$downloadUrl"
unzip -p RepoResults14.zip RepoResults14/elastic.kibana.replay.txt > elastic.kibana.replay.txt
npm install --no-save @typescript/server-replayTo run the repro: # `npx tsreplay --help` to learn about helpful switches for debugging, logging, etc.
npx tsreplay ./kibana ./elastic.kibana.replay.txt <PATH_TO_tsserver.js> |
|
@rbuckton Here are the results of running the top 400 repos comparing Everything looks good! |
It doesn't seem like the crash is related? If I run the repro locally I have the same result with or without this change. I'll try running it again in case it was a quirk. @typescript-bot test tsserver top100 |
|
Temporarily reverting change to validate crash. @typescript-bot test tsserver top100 |
|
The server tests have issues; I don't think they properly detect old/new crashes like the tsc tests yet. |
|
@rbuckton Here are the results of running the user tests comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
@rbuckton Here are the results of running the top 200 repos comparing Something interesting changed - please have a look. Details
|
|
@rbuckton Here are the results of running the top 200 repos comparing Something interesting changed - please have a look. DetailsServer exited prematurely with code unknown and signal SIGABRTAffected reposbackstage/backstageRaw error text:RepoResults8/backstage.backstage.rawError.txt in the artifact folder Replay commands: RepoResults8/backstage.backstage.replay.txt in the artifact folder
Last few requests{"seq":109,"type":"request","command":"definitionAndBoundSpan","arguments":{"file":"@PROJECT_ROOT@/scripts/assemble-manifest.js","line":62,"offset":51}}
{"seq":110,"type":"request","command":"definitionAndBoundSpan","arguments":{"file":"@PROJECT_ROOT@/scripts/assemble-manifest.js","line":76,"offset":1}}
{"seq":111,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/scripts/prepare-release.js"],"openFiles":[]}}
{"seq":112,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/plugins/search-backend-module-catalog/config.d.ts","projectRootPath":"@PROJECT_ROOT@"}]}}
Repro steps#!/bin/bash
git clone https://github.com/backstage/backstage --recurse-submodules
git -C "./backstage" reset --hard 024b530575d5d9d4d28d53db56d245d81a8c413c
# Install packages (exact steps are below, but it might be easier to follow the repo readme)
yarn --cwd "/mnt/ts_downloads/base/backstage" install --no-immutable --mode=skip-build
yarn --cwd "/mnt/ts_downloads/base/backstage/storybook" install --no-immutable --mode=skip-build
yarn --cwd "/mnt/ts_downloads/base/backstage/microsite" install --no-immutable --mode=skip-build
downloadUrl=$(curl -s "https://typescript.visualstudio.com/TypeScript/_apis/build/builds/161881/artifacts?artifactName=RepoResults8&api-version=7.0" | jq -r ".resource.downloadUrl")
wget -O RepoResults8.zip "$downloadUrl"
unzip -p RepoResults8.zip RepoResults8/backstage.backstage.replay.txt > backstage.backstage.replay.txt
npm install --no-save @typescript/server-replayTo run the repro: # `npx tsreplay --help` to learn about helpful switches for debugging, logging, etc.
npx tsreplay ./backstage ./backstage.backstage.replay.txt <PATH_TO_tsserver.js>Server exited prematurely with code unknown and signal SIGABRTAffected reposcalcom/cal.comRaw error text:RepoResults7/calcom.cal.com.rawError.txt in the artifact folder Replay commands: RepoResults7/calcom.cal.com.replay.txt in the artifact folder
Last few requests{"seq":89,"type":"request","command":"navto","arguments":{"searchValue":"a1R","maxResultCount":256}}
{"seq":90,"type":"request","command":"updateOpen","arguments":{"changedFiles":[{"fileName":"@PROJECT_ROOT@/packages/trpc/index.ts","textChanges":[{"newText":" //comment","start":{"line":1,"offset":26},"end":{"line":1,"offset":26}}]}],"closedFiles":[],"openFiles":[]}}
{"seq":91,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/packages/types/Credential.d.ts"],"openFiles":[]}}
{"seq":92,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/packages/prisma/seed-utils.ts","projectRootPath":"@PROJECT_ROOT@"}]}}
Repro steps#!/bin/bash
git clone https://github.com/calcom/cal.com --recurse-submodules
git -C "./cal.com" reset --hard 01933e5c15ec62949a6d74942a7b75a73e3da76b
yarn --cwd "/mnt/ts_downloads/base/cal.com" install --no-immutable --mode=skip-build
downloadUrl=$(curl -s "https://typescript.visualstudio.com/TypeScript/_apis/build/builds/161881/artifacts?artifactName=RepoResults7&api-version=7.0" | jq -r ".resource.downloadUrl")
wget -O RepoResults7.zip "$downloadUrl"
unzip -p RepoResults7.zip RepoResults7/calcom.cal.com.replay.txt > calcom.cal.com.replay.txt
npm install --no-save @typescript/server-replayTo run the repro: # `npx tsreplay --help` to learn about helpful switches for debugging, logging, etc.
npx tsreplay ./cal.com ./calcom.cal.com.replay.txt <PATH_TO_tsserver.js>Server exited prematurely with code unknown and signal SIGABRTAffected reposelastic/kibanaRaw error text:RepoResults14/elastic.kibana.rawError.txt in the artifact folder Replay commands: RepoResults14/elastic.kibana.replay.txt in the artifact folder
Last few requests{"seq":2070,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/x-pack/plugins/serverless_search/public/application/elasticsearch.tsx","line":15,"offset":40,"includeExternalModuleExports":false,"triggerKind":1}}
{"seq":2071,"type":"request","command":"completionEntryDetails","arguments":{"file":"@PROJECT_ROOT@/x-pack/plugins/serverless_search/public/application/elasticsearch.tsx","line":15,"offset":40,"entryNames":["@kbn/shared-ux-router"]}}
{"seq":2072,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/x-pack/test/alerting_api_integration/common/lib/task_manager_utils.ts"],"openFiles":[]}}
{"seq":2073,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/x-pack/plugins/security_solution_serverless/public/navigation/management_cards.ts","projectRootPath":"@PROJECT_ROOT@"}]}}
Repro steps#!/bin/bash
git clone https://github.com/elastic/kibana --recurse-submodules
git -C "./kibana" reset --hard 2ec4ec362d21fe03d04d53181fa0a8f7da888eb5
downloadUrl=$(curl -s "https://typescript.visualstudio.com/TypeScript/_apis/build/builds/161881/artifacts?artifactName=RepoResults14&api-version=7.0" | jq -r ".resource.downloadUrl")
wget -O RepoResults14.zip "$downloadUrl"
unzip -p RepoResults14.zip RepoResults14/elastic.kibana.replay.txt > elastic.kibana.replay.txt
npm install --no-save @typescript/server-replayTo run the repro: # `npx tsreplay --help` to learn about helpful switches for debugging, logging, etc.
npx tsreplay ./kibana ./elastic.kibana.replay.txt <PATH_TO_tsserver.js> |
|
The crashes seem to be unrelated as they're still happening even after reverting the change. |
|
@typescript-bot test tsserver top100 |
gabritto
reviewed
May 29, 2024
| : getIterationTypeOfGeneratorFunctionReturnType(IterationTypeKind.Yield, contextualReturnType, isAsyncGenerator); | ||
| if (node.asteriskToken) { | ||
| const iterationTypes = getIterationTypesOfGeneratorFunctionReturnType(contextualReturnType, isAsyncGenerator); | ||
| const yieldType = iterationTypes?.yieldType ?? silentNeverType; |
There was a problem hiding this comment.
Why do we fall back to silentNeverType?
There was a problem hiding this comment.
We need to fall back to something we do not infer from, and a generator that doesn't yield otherwise infers a T of never. silentNeverType is non-inferable, so it seemed like the correct choice.
|
@rbuckton Here are the results of running the top 200 repos comparing Something interesting changed - please have a look. DetailsServer exited prematurely with code unknown and signal SIGABRTAffected reposcalcom/cal.comRaw error text:RepoResults7/calcom.cal.com.rawError.txt in the artifact folder Replay commands: RepoResults7/calcom.cal.com.replay.txt in the artifact folder
Last few requests{"seq":656,"type":"request","command":"navtree","arguments":{"file":"@PROJECT_ROOT@/packages/features/get-cal-video-reference.test.ts"}}
{"seq":657,"type":"request","command":"navbar","arguments":{"file":"@PROJECT_ROOT@/packages/features/get-cal-video-reference.test.ts"}}
{"seq":658,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":["@PROJECT_ROOT@/packages/lib/CloseCom.ts"],"openFiles":[]}}
{"seq":659,"type":"request","command":"updateOpen","arguments":{"changedFiles":[],"closedFiles":[],"openFiles":[{"file":"@PROJECT_ROOT@/packages/debugging/index.tsx","projectRootPath":"@PROJECT_ROOT@"}]}}
Repro steps#!/bin/bash
git clone https://github.com/calcom/cal.com --recurse-submodules
git -C "./cal.com" reset --hard 5f082e11f8c82478d3e0ffbb23c65e28bc1f08d4
yarn --cwd "/mnt/ts_downloads/base/cal.com" install --no-immutable --mode=skip-build
downloadUrl=$(curl -s "https://typescript.visualstudio.com/TypeScript/_apis/build/builds/161974/artifacts?artifactName=RepoResults7&api-version=7.0" | jq -r ".resource.downloadUrl")
wget -O RepoResults7.zip "$downloadUrl"
unzip -p RepoResults7.zip RepoResults7/calcom.cal.com.replay.txt > calcom.cal.com.replay.txt
npm install --no-save @typescript/server-replayTo run the repro: # `npx tsreplay --help` to learn about helpful switches for debugging, logging, etc.
npx tsreplay ./cal.com ./calcom.cal.com.replay.txt <PATH_TO_tsserver.js> |
gabritto
approved these changes
May 29, 2024
skeate
added a commit
to skeate/TypeScript
that referenced
this pull request
Jun 1, 2024
* upstream/main: (37 commits) Added NoTruncation flag to completions (microsoft#58719) Clone node to remove location even when it has been modified if needed (microsoft#58706) Properly account for `this` argument in intersection apparent type caching (microsoft#58677) Fix: Include Values of Script Extensions for Unicode Property Value Expressions in Regular Expressions (microsoft#58615) In `reScanSlashToken` use `charCodeChecked` not `codePointChecked` (microsoft#58727) Shorten error spans for errors reported on constructor declarations (microsoft#58061) Mark file as skips typechecking if it contains ts-nocheck (microsoft#58593) Fixed an issue with broken `await using` declarations in `for of` loops (microsoft#56466) Do not expand type references in keyof and index access (microsoft#58715) Improve the performance of isolatedDeclarations quickfix (microsoft#58722) Unwrap `NoInfer` types when narrowing (microsoft#58292) Recover from type reuse errors by falling back to inferred type printing (microsoft#58720) Fixing self import (microsoft#58718) Enable JS emit for noCheck and noCheck for transpileModule (microsoft#58364) Revert PR 55371 (microsoft#58702) Update dependencies (microsoft#58639) Fix baselines after PR 58621 (microsoft#58705) Do not infer `yield*` type from contextual `TReturn` (microsoft#58621) `await using` normative changes (microsoft#58624) Handling statements from a known source file (microsoft#58679) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the contextual type we produce for the operand to
yield*so that we do not consider theTReturntype of the containing generator's return type as a source of inference since theTReturnof a generator has no bearing on the result ofyield*. Instead, we substitute the contextual type for the entireyield*expression, e.g.:At (a),
Ushould be inferred fromT(i.e.,number) sincefwill delegateyieldfromg.At (b),
UReturnshould not be inferred fromTReturn(i.e.,string) sincefdoes not delegate the return value ofg. Sincev1is untyped,UReturnis inferred asunknown.At (c),
Ushould be inferred fromT(i.e.,number) sincefwill delegateyieldfromg.At (d),
UReturnshould not be inferred fromTReturn(i.e.,string) sincefdoes not delegate the return value ofg. Sincev2has the typePoint,UReturnis inferred asPoint.Fixes #57903