fix: avoid downleveled dynamic import closing over specifier expression#49663
Conversation
typescript-bot
added
the
For Milestone Bug
|
I realized Babel is transpiling it to |
Is there a reason to do that over what you're already doing in this PR? I'm not so concerned about the temp variable in this case. |
Josh-Cena
changed the title
|
No—no adverse effects that I can think of (apart from some micro-optimizations, maybe?) If you think they both work I'll stay with the current solution. |
| var _a; | ||
| return _a = this._path, __syncRequire ? Promise.resolve().then(() => require(_a)) : new Promise((resolve_1, reject_1) => { require([_a], resolve_1, reject_1); }); | ||
| var _a, _b; | ||
| return _a = this._path, __syncRequire ? (_b = _a, Promise.resolve().then(() => require(_b))) : new Promise((resolve_1, reject_1) => { require([_a], resolve_1, reject_1); }); |
There was a problem hiding this comment.
The additional aliasing here is unnecessary.
There was a problem hiding this comment.
You're correct. However I'm not sure if it's possible to be fixed on a generic level, since the temp variable created from the UMD transformer may be re-assigned. I've committed the new baselines in 3baf06d with the two unnecessary aliases fixed, but there are some other places where it looks suspicious. I can't really read the output to tell if it's unsafe, so I'll appreciate if you can help to review here.
Never mind, the temp variable will never be re-assigned because it isn't leaking anywhere
| var _a, _b; | ||
| const packageName = '.'; | ||
| const packageJson = yield (_a = packageName + '/package.json', __syncRequire ? Promise.resolve().then(() => require(_a)) : new Promise((resolve_1, reject_1) => { require([_a], resolve_1, reject_1); })); | ||
| const packageJson = yield (_a = packageName + '/package.json', __syncRequire ? (_b = _a, Promise.resolve().then(() => require(_b))) : new Promise((resolve_1, reject_1) => { require([_a], resolve_1, reject_1); })); |
There was a problem hiding this comment.
The additional aliasing here is unnecessary.
Co-authored-by: Ron Buckton <[email protected]>
| _a = (...["PathModule"]), Promise.resolve().then(() => require(_a)); | ||
| var p1 = (_b = (...a), Promise.resolve().then(() => require(_b))); |
There was a problem hiding this comment.
This is invalid grammar, but the input is invalid grammar as well. I assume we can safely treat it as garbage in, garbage out?
4 participants
Fixes #49652
Fixes #48285