Skip to content

Bug in async awaits in es5 #40614

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

Closed
JustynaBroniszewska opened this issue Sep 17, 2020 · 13 comments · Fixed by #41079
Closed

Bug in async awaits in es5 #40614

JustynaBroniszewska opened this issue Sep 17, 2020 · 13 comments · Fixed by #41079
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@JustynaBroniszewska
Copy link

TypeScript Version: 4.1.0-dev.20200917

Search Terms: is:issue is:open async label:bug change

Code

async function bar () {
  return {
    a: await Promise.resolve(1),
    b: await Promise.resolve(2),
    c: await Promise.resolve(3),
    d: await Promise.resolve(4),
    e: await Promise.resolve(5),
    f: await Promise.resolve(6),
    g: await Promise.resolve(7),
    h: await Promise.resolve(8),
    // works correctly if you comment the next line
    i: await Promise.resolve(9),
  }
}

bar().then(console.log)

tsconfig.json

{
  "compilerOptions": {
    "lib": ["DOM", "ES2015"]
  }
}

Expected behavior:

{ a: 1, b: 2, c: 3, d: 4, e: 5, f: 6, g: 7, h: 8, i: 9 }

Actual behavior:

{ a: 9, b: 9, c: 9, d: 9, e: 9, f: 9, g: 9, h: 9, i: 9 }

Playground Link:
https://www.typescriptlang.org/play?target=1#code/IYZwngdgxgBAZgV2gFwJYHsIwEbAE4wAUAlDAN4BQMMeApsgnlpddcAFwzADuwqyMAAp50AW1QhaAOjoh0AGwButQgEZiAGiqtsnHnwHCxE6bIXLCAJk3bqUPb35CR4yTNpylKgMw3WMABMHA2djNzMvQgAWP1ZaYKcjV1MPcxUAVljqOATDFxN3TwsANiyYAHNc0OTCtMIAdjKACyqkgoiLAA4y1Fb88NTIgE5YgF8KcYpcPBIpZCbaCEIoTE9peXRy4iA

Related Issues:
#40047

@j-oliveras
Copy link
Contributor

The cause seems to be that the generated code use the same value to init all properties (case 9):

// Excluded __awaiter and __generator functions
function bar() {
    return __awaiter(this, void 0, void 0, function () {
        var _a;
        return __generator(this, function (_b) {
            switch (_b.label) {
                case 0: return [4 /*yield*/, Promise.resolve(1)];
                case 1: return [4 /*yield*/, Promise.resolve(2)];
                case 2: return [4 /*yield*/, Promise.resolve(3)];
                case 3: return [4 /*yield*/, Promise.resolve(4)];
                case 4: return [4 /*yield*/, Promise.resolve(5)];
                case 5: return [4 /*yield*/, Promise.resolve(6)];
                case 6: return [4 /*yield*/, Promise.resolve(7)];
                case 7: return [4 /*yield*/, Promise.resolve(8)];
                case 8: return [4 /*yield*/, Promise.resolve(9)];
                case 9: return [2 /*return*/, (_a = {},
                        _a.a = _b.sent(),
                        _a.b = _b.sent(),
                        _a.c = _b.sent(),
                        _a.d = _b.sent(),
                        _a.e = _b.sent(),
                        _a.f = _b.sent(),
                        _a.g = _b.sent(),
                        _a.h = _b.sent(),
                        _a.i = _b.sent(),
                        _a)];
            }
        });
    });
}
bar().then(console.log);

@IllusionMH
Copy link
Contributor

IllusionMH commented Sep 17, 2020

At first I thought that this is problem with generator transformations, but looks like version where async changed to generator function and await changed to yield provides correct assignments in each case in compiled JS.

Also original problem is not reproducible in 3.9.7.

@j-oliveras
Copy link
Contributor

The nightly have the same problem.

As a reference for the typescript team, in version 3.9.7 it generates:

// Excluded __awaiter and __generator functions
function bar() {
    return __awaiter(this, void 0, void 0, function () {
        var _a;
        return __generator(this, function (_b) {
            switch (_b.label) {
                case 0:
                    _a = {};
                    return [4 /*yield*/, Promise.resolve(1)];
                case 1:
                    _a.a = _b.sent();
                    return [4 /*yield*/, Promise.resolve(2)];
                case 2:
                    _a.b = _b.sent();
                    return [4 /*yield*/, Promise.resolve(3)];
                case 3:
                    _a.c = _b.sent();
                    return [4 /*yield*/, Promise.resolve(4)];
                case 4:
                    _a.d = _b.sent();
                    return [4 /*yield*/, Promise.resolve(5)];
                case 5:
                    _a.e = _b.sent();
                    return [4 /*yield*/, Promise.resolve(6)];
                case 6:
                    _a.f = _b.sent();
                    return [4 /*yield*/, Promise.resolve(7)];
                case 7:
                    _a.g = _b.sent();
                    return [4 /*yield*/, Promise.resolve(8)];
                case 8:
                    _a.h = _b.sent();
                    return [4 /*yield*/, Promise.resolve(9)];
                case 9: return [2 /*return*/, (_a.i = _b.sent(),
                        _a)];
            }
        });
    });
}
bar().then(console.log);

@IllusionMH
Copy link
Contributor

Interesting enough @andrewpg found that it doesn't depend on number of await's
but on number of object keys after first await.

Example

@IllusionMH
Copy link
Contributor

Caused by 8231519 in #38880

/cc @elibarzilay @rbuckton

@andrewbranch andrewbranch added the Bug A bug in TypeScript label Sep 18, 2020
@andrewbranch andrewbranch added this to the TypeScript 4.1.1 milestone Sep 18, 2020
@andrewbranch
Copy link
Member

Thanks for the investigation, everyone! 🌟

@styu
Copy link

styu commented Sep 18, 2020

Would it be possible to backport a fix for this to 4.0.x? We unfortunately had to revert our 4.0.x upgrade because this issue was causing our apps to crash, and we'd love to upgrade to 4.0.x before November :)

@DanielRosenwasser
Copy link
Member

Yes, I think we can packport the eventual fix to TypeScript 4.0.4. @elibarzilay do you think you'd be able to prioritize this issue?

@DanielRosenwasser
Copy link
Member

Seems like @elibarzilay is having hardware issues. @rbuckton can you take a look at this?

@elibarzilay
Copy link
Contributor

I'm back, kind of. Trying to find it now.

elibarzilay added a commit to elibarzilay/TypeScript that referenced this issue Oct 13, 2020
Uses essentially the same code as `visitCommaExpression` (which was
moved, to keep both together and close to
`visit{Right,Left}AssociativeBinaryExpression`).

Fixes microsoft#40614.
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Oct 13, 2020
elibarzilay added a commit to elibarzilay/TypeScript that referenced this issue Oct 14, 2020
Uses essentially the same code as `visitCommaExpression` (which was
moved, to keep both together and close to
`visit{Right,Left}AssociativeBinaryExpression`).

Fixes microsoft#40614.
elibarzilay added a commit that referenced this issue Oct 14, 2020
Uses essentially the same code as `visitCommaExpression` (which was
moved, to keep both together and close to
`visit{Right,Left}AssociativeBinaryExpression`).

Fixes #40614.
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Oct 14, 2020

If folks can try out the next nightly version of TypeScript to validate that the fix works for them, we'd appreciate that a ton!

@styu
Copy link

styu commented Oct 16, 2020

@DanielRosenwasser I tried the nightly out in our repo and it seems to have fixed the issue!

@styu
Copy link

styu commented Oct 19, 2020

@DanielRosenwasser sorry to bother, but what's the timeline for 4.0.4? We're super eager to upgrade to 4.0.x :)

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this issue Oct 22, 2020
Component commits:
6fa32ba `transformGenerators`: handle `CommaListExpression`
Uses essentially the same code as `visitCommaExpression` (which was
moved, to keep both together and close to
`visit{Right,Left}AssociativeBinaryExpression`).

Fixes microsoft#40614.
DanielRosenwasser added a commit that referenced this issue Oct 22, 2020
* Cherry-pick PR #41079 into release-4.0

Component commits:
6fa32ba `transformGenerators`: handle `CommaListExpression`
Uses essentially the same code as `visitCommaExpression` (which was
moved, to keep both together and close to
`visit{Right,Left}AssociativeBinaryExpression`).

Fixes #40614.

* Accepted baselines.

Co-authored-by: Eli Barzilay <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants