Open blickly opened 3 years ago
The __read
helper was used even though the target was ESNext due to --downlevelIteration
being enabled. However, it does seem like --importHelpers
should have imported it instead of inlining it. Interestingly, if another helper is used, __read
gets imported along with it: https://www.typescriptlang.org/play?downlevelIteration=true&importHelpers=true&target=7&module=1#code/CYUwxgNghgTiAEAzArgOzAFwJYHtVJxwAoBKALnilQE8BuAKBAA8AHHGDeMPAZ04G0AggBp4ANQC68ALwFiJBvSwBbNh3gAqSj3gAjaAAskMHMvgByRIXO14AejtdTykKk45knDAaw6IWVBB6fSgDBiA
@rbuckton, is this intentional, or is __read
just forgetting to signal that the helpers need to be imported?
I'm happy to change my flags if I can both avoid the helpers and get correct behavior. Unfortunately, when I turn off --downlevelIteration
, I get incorrect behavior that doesn't work with iterables:
"use strict";
var _a;
Object.defineProperty(exports, "__esModule", { value: true });
exports.V = exports.A = void 0;
_a = foo(), exports.A = _a[0], exports.V = _a[1];
We use the __read
helper to downlevel the export, even though you're using --target esnext
, because you've specified --module commonjs
. This should be importing it from tslib
if --importHelpers
is used, since the file is a module.
We could also investigate updating the module.ts
transformer to rewrite the output as follows, when using --target ES2015
or higher:
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.V = exports.A = void 0;
[exports.A, exports.V] = foo();
I think we do the general downleveling for the sake of emit simplicity, rather than --target
consistency.
Here's an interesting variation:
declare function foo(): any;
const [A, V] = foo();
export { A, V }
Changing the export syntax prevents the downleveling.
@rbuckton, I agree that in the long term, we should eventually switch from --module commonjs
to --module esnext
, at which point I assume that the --downlevelIteration
will become a no-op since the tslib
helper methods will all be unneeded (I hope).
For the time being, is there any guidance on which code patterns will need to depend on the tslib
helpers in the output?
I have the same problem with a for-of loop, but I think I found the trick:
script1.ts:
const array = [1, 2, 3];
for (const value of array) {
console.log(value)
}
will be transpiled to
var __values = (this && this.__values) || function(o) {
var s = typeof Symbol === "function" && Symbol.iterator, m = s && o[s], i = 0;
if (m) return m.call(o);
if (o && typeof o.length === "number") return {
next: function () {
if (o && i >= o.length) o = void 0;
return { value: o && o[i++], done: !o };
}
};
throw new TypeError(s ? "Object is not iterable." : "Symbol.iterator is not defined.");
};
var e_1, _a;
var array = [1, 2, 3];
try {
for (var array_1 = __values(array), array_1_1 = array_1.next(); !array_1_1.done; array_1_1 = array_1.next()) {
var value = array_1_1.value;
console.log(value);
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (array_1_1 && !array_1_1.done && (_a = array_1.return)) _a.call(array_1);
}
finally { if (e_1) throw e_1.error; }
}
//# sourceMappingURL=test2.js.map
script2.ts:
export function fuu() {
const array = [1, 2, 3];
for (const value of array) {
console.log(value)
}
}
fuu();
will be correctly transpiled to
Object.defineProperty(exports, "__esModule", { value: true });
exports.fuu = void 0;
var tslib_1 = require("tslib");
function fuu() {
var e_1, _a;
var array = [1, 2, 3];
try {
for (var array_1 = tslib_1.__values(array), array_1_1 = array_1.next(); !array_1_1.done; array_1_1 = array_1.next()) {
var value = array_1_1.value;
console.log(value);
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (array_1_1 && !array_1_1.done && (_a = array_1.return)) _a.call(array_1);
}
finally { if (e_1) throw e_1.error; }
}
}
exports.fuu = fuu;
fuu();
//# sourceMappingURL=test1.js.map
so the difference is that in the second script, which imports tslib (this is what is expected), the code is wrapped by a function. In the first script the code is in the global scope and then typescript emits the helper functions as expected.
If you remove exort
from script2 it breaks. so the key seems to be the export keyword?
Hi there. I started looking into this. Here is my current understanding of why this happens. This is roughly the order of transformations in the compilation process:
This means that importHelpers
is generally respected. The only case where it's ignored is when the helper is required in an import/export statement and it's the only required helper in the file.
I see 2 ways to fix this:
Is my understanding correct? And which option would you prefer?
I can try to work on a PR. But would like to make sure I go in the right direction 🙂
From a user's perspective, I think it would be nicer if the module transformation didn't depend on helpers that are not used in the --target
level that was requested.
But I think folks on the MS side might have a preference on which approach would fit better with the TSC architecture.
Specifying --target ESNext
does not necessarily mean there is no transpilation. What you are seeing is a side-effect of using --target commonjs
and export const [A, V] = foo()
. This needs to be transpiled to assignments to exports.A
and exports.V
. While we could possibly transpile this to [exports.A, exports.V] = foo()
, the module transformer happens at the end of all of the other transformation passes and needs to work regardless as to the --target
you specify. Rather than duplicate the work we do in the earlier language-specific transformers, we opted to instead perform a general destructuring transformation that is --target
agnostic.
Any change here would require branching on the target ECMAScript edition to control how much of the destructuring transform we actually need.
Aside from this, there's no other reason to use --downlevelIteration
other than for --target ES5
. I'd considered closing this as "Working as Intended", but I'd rather make it an error to use --downlevelIteration
with the other script targets. To do that means addressing the few cases where it matters in our module transformers.
Bug Report
🔎 Search Terms
array destructuring
downLevelIteration
importHelpers
ESNEXT CommonJS🕗 Version & Regression Information
importHelpers
⏯ Playground Link
Playground link with relevant code
Note: I'm not sure how to reliably get the
target
option to encode into the URL; you may need to set the target manually toESNext
(or any other value >=ES2015
)💻 Code
🙁 Actual behavior
The
__read
transpilation helper was injected into the output JS, even though I specified that I didn't want transpilation (throughtarget=ESNEXT
). Additionally, the transpilation helper was injected inline even though I specified theimportHelpers=true
option.🙂 Expected behavior
I was expecting output that leveraged the
target=ESNEXT
setting to include the array destructuring in the output code, like: