Skip to content

Conversation

@rbuckton
Copy link
Contributor

In #49663 we fixed one issue related to closures and dynamic import, but introduced another. This changes the emit to use an IIFE to ensure the value of the variable is captured properly. We could potentially drop back to the _a = x, Promise.resolve().then(...) emit if we're not in any loop, but that would require tracking iteration statements as we descend, and dynamic import isn't likely to be used in a fast path that cares about the overhead of an IIFE.

Fixes #51554.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Nov 17, 2022
@Josh-Cena
Copy link
Contributor

@rbuckton What do you think about the fix proposed in #51562? Frankly, I don't see the point to emit another closure just so that the temp variable works—directly coercing it to string in the current scope seems like more sensible behavior.

@rbuckton rbuckton closed this Nov 20, 2022
@rbuckton
Copy link
Contributor Author

Closed in favor of #51562

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4.9.3 commonjs output regression: Dynamical path import()s inside sync for-of are all called with the same parameter

4 participants