Support top level "for await of" #37424

hikarino-my · 2020-03-16T21:51:30Z

This PR will add supports for top level "for await of".

Fixes #37402

sandersn · 2020-03-25T20:19:22Z

Reviewers list is broken, but @elibarzilay maybe you can look at this too.

elibarzilay · 2020-07-17T22:33:49Z

(Rebased & resolved conflicts)

elibarzilay

Looks like it should have more tests... Minor comments too.

Also, it would be good to verify that everything is fine after I rebased, since I needed to resolve conflicts.

src/compiler/checker.ts

tests/baselines/reference/topLevelAwaitNonModule.errors.txt

hikarino-my · 2020-07-26T18:55:31Z

@elibarzilay
Thank you for your review!
I've applied your suggestions to the code and added a test case so that it will check for positive case.

src/compiler/checker.ts

hikarino-my · 2020-08-05T11:05:26Z

src/compiler/checker.ts

+                            const diagnostic = createDiagnosticForNode(forInOrOfStatement.awaitModifier, Diagnostics.A_for_await_of_statement_is_only_allowed_within_an_async_function_or_async_generator);
+                            const func = getContainingFunction(forInOrOfStatement);
+                            if (func && func.kind !== SyntaxKind.Constructor) {
+                                Debug.assert((getFunctionFlags(func) & FunctionFlags.Async) === 0, "Enclosing function should never be an async function.");
+                                const relatedInfo = createDiagnosticForNode(func, Diagnostics.Did_you_mean_to_mark_this_function_as_async);
+                                addRelatedInfo(diagnostic, relatedInfo);
+                            }
+                            diagnostics.add(diagnostic);


Since diagnostic is used twice here, didn't changed this section.

hikarino-my · 2021-01-15T19:05:51Z

@elibarzilay @rbuckton @elibarzilay
Well, being kept waiting for +5 months with no review, currently i have no interest in this PR nor this project.
Since I'm away from this project for a long time, I'm not sure this works now.
I'll wait for your responses for a week but if there isn't i'm thinking of closing this PR.

rbuckton · 2021-01-15T19:39:50Z

src/compiler/checker.ts

+                        if (!hasParseDiagnostics(sourceFile)) {
+                            if (!isEffectiveExternalModule(sourceFile, compilerOptions)) {
+                                diagnostics.add(createDiagnosticForNode(forInOrOfStatement.awaitModifier,
+                                    Diagnostics.await_expressions_are_only_allowed_at_the_top_level_of_a_file_when_that_file_is_a_module_but_this_file_has_no_imports_or_exports_Consider_adding_an_empty_export_to_make_this_file_a_module));


@DanielRosenwasser would we want to reuse the phrase "await expressions" here, or should we have a more specific message for for await of?

Create a new diagnostic so we don't seem janky and lazy

'for await' loops are only allowed at the top level of a file when that file is a module, but this file...

rbuckton

This looks good to me. We may need to make the diagnostic message a little more generic to cover both await x and for-await-of, but we could easily do that in a follow-up PR if necessary.

rbuckton · 2021-01-20T19:39:04Z

@typescript-bot test this
@typescript-bot user test this

typescript-bot · 2021-01-20T19:39:08Z

Heya @rbuckton, I've started to run the parallelized community code test suite on this PR at d8a1db2. You can monitor the build here.

typescript-bot · 2021-01-20T19:39:09Z

Heya @rbuckton, I've started to run the extended test suite on this PR at d8a1db2. You can monitor the build here.

rbuckton · 2021-01-21T02:40:48Z

The RWC test suite looks good (failures are unrelated to this change). We're having some issues with our user test suite currently, but from what I can tell things look good here.

@DanielRosenwasser - Will we consider taking this for 4.2 RC, or should we wait to merge until after RC?

rbuckton · 2021-01-21T02:43:50Z

To clarify, this loosens a restriction specific to cases where we should allow it. It has no effect on run-time emit and would only remove errors that we shouldn't be reporting (we will still report errors for incorrect for-await-of in a script or in a --target/--module combination that doesn't support top-level await).

rbuckton · 2021-01-25T21:40:40Z

I spoke with @DanielRosenwasser offline and we will be taking this for our 4.2 RC. Thanks @hikarinotomadoi for the contribution!

hikarino-my mentioned this pull request Mar 16, 2020

Top Level "for await" not supported, but should be #37402

Closed

hikarino-my force-pushed the fix37402 branch from 33ece12 to 9bb35c8 Compare March 16, 2020 22:15

sandersn assigned rbuckton Mar 25, 2020

sandersn added the For Backlog Bug PRs that fix a backlog bug label Mar 27, 2020

Vandivier mentioned this pull request Apr 4, 2020

Remove extraneous top-level await requirement #37794

Closed

DanielRosenwasser requested review from elibarzilay and rbuckton July 15, 2020 21:13

elibarzilay force-pushed the fix37402 branch from 9bb35c8 to 5a711e7 Compare July 17, 2020 22:32

elibarzilay suggested changes Jul 17, 2020

View reviewed changes

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

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

tests/baselines/reference/topLevelAwaitNonModule.errors.txt Outdated Show resolved Hide resolved

hikarino-my requested a review from elibarzilay July 26, 2020 18:33

elibarzilay reviewed Aug 3, 2020

View reviewed changes

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

hikarino-my commented Aug 5, 2020

View reviewed changes

hikarino-my added 5 commits January 15, 2021 13:55

Support Top Level "for await of".

8b1a8c3

Add test cases for top level "for await of".

33cce67

Apply suggestions from code review

9356665

add test cases

d2b82c3

remove redundant variables

3286e9c

hikarino-my force-pushed the fix37402 branch from 686bd87 to 3286e9c Compare January 15, 2021 04:58

hikarino-my requested a review from elibarzilay January 15, 2021 04:58

fix test baselines

d8a1db2

rbuckton reviewed Jan 15, 2021

View reviewed changes

rbuckton approved these changes Jan 20, 2021

View reviewed changes

Update diagnostic message and tests

297f092

rbuckton merged commit 701493f into microsoft:master Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support top level "for await of" #37424

Support top level "for await of" #37424

hikarino-my commented Mar 16, 2020

sandersn commented Mar 25, 2020

elibarzilay commented Jul 17, 2020

elibarzilay left a comment

hikarino-my commented Jul 26, 2020

hikarino-my Aug 5, 2020

hikarino-my commented Jan 15, 2021

rbuckton Jan 15, 2021

DanielRosenwasser Jan 20, 2021

rbuckton left a comment

rbuckton commented Jan 20, 2021

typescript-bot commented Jan 20, 2021 •

edited

Loading

typescript-bot commented Jan 20, 2021 •

edited

Loading

rbuckton commented Jan 21, 2021

rbuckton commented Jan 21, 2021 •

edited

Loading

rbuckton commented Jan 25, 2021

Support top level "for await of" #37424

Support top level "for await of" #37424

Conversation

hikarino-my commented Mar 16, 2020

sandersn commented Mar 25, 2020

elibarzilay commented Jul 17, 2020

elibarzilay left a comment

Choose a reason for hiding this comment

hikarino-my commented Jul 26, 2020

hikarino-my Aug 5, 2020

Choose a reason for hiding this comment

hikarino-my commented Jan 15, 2021

rbuckton Jan 15, 2021

Choose a reason for hiding this comment

DanielRosenwasser Jan 20, 2021

Choose a reason for hiding this comment

rbuckton left a comment

Choose a reason for hiding this comment

rbuckton commented Jan 20, 2021

typescript-bot commented Jan 20, 2021 • edited Loading

typescript-bot commented Jan 20, 2021 • edited Loading

rbuckton commented Jan 21, 2021

rbuckton commented Jan 21, 2021 • edited Loading

rbuckton commented Jan 25, 2021

typescript-bot commented Jan 20, 2021 •

edited

Loading

typescript-bot commented Jan 20, 2021 •

edited

Loading

rbuckton commented Jan 21, 2021 •

edited

Loading