Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat 'yield;' as 'yield undefined;' #22297

Merged
3 commits merged into from
Mar 8, 2018
Merged

Treat 'yield;' as 'yield undefined;' #22297

3 commits merged into from
Mar 8, 2018

Conversation

ghost
Copy link

@ghost ghost commented Mar 2, 2018

Fixes #22290
Previously we would skip over yield; with no argument; now we will treat it as yielding undefined.

@ghost ghost requested review from sandersn, weswigham and rbuckton March 2, 2018 17:05
@ghost ghost changed the title Treat 'yeild;' as 'yield undefined;' Treat 'yield;' as 'yield undefined;' Mar 2, 2018
==== tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck48.ts (1 errors) ====
function* g() {
~
!!! error TS7025: Generator implicitly has type 'IterableIterator<any>' because it does not yield any values. Consider supplying a return type.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the correct error. without noImplicitAny, it should be an implicit any and not undefined.

@ghost ghost force-pushed the yieldNothing branch from 3e7a481 to 9dac65f Compare March 2, 2018 21:13
@ghost ghost force-pushed the yieldNothing branch from 9dac65f to f910468 Compare March 2, 2018 23:50
@ghost ghost mentioned this pull request Mar 3, 2018
return aggregatedTypes;
}

function getYieldedTypeOfYieldExpression(node: YieldExpression, isAsync: boolean, checkMode?: CheckMode): Type {
const errorNode = node.expression || node;
const expressionType = node.expression ? checkExpressionCached(node.expression, checkMode) : undefinedWideningType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the refactoring that is part of this PR -- it make the code much easier to read. But I want to point out that this is the only line needed to fix the bug. The bug would get fixed a lot faster if we could just review a one-line fix and look at the refactor separately.

@ghost ghost merged commit e48bcd6 into master Mar 8, 2018
@ghost ghost deleted the yieldNothing branch March 8, 2018 23:41
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants