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

Use IIFE to capture variable for transformed dynamic import #51581

Closed
wants to merge 1 commit into from

Conversation

rbuckton
Copy link
Member

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
Member Author

Closed in favor of #51562

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
3 participants