Skip to content

Scoping issue with closure in default parameters when transpiling to ES5 #17842

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
jlowcs opened this issue Aug 16, 2017 · 3 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@jlowcs
Copy link

jlowcs commented Aug 16, 2017

TypeScript Version: 2.4.0

Code

(function foo(x, f = () => x) { var x; x++; console.log(x, f()); })(42)

Expected behavior:

console output: 43 42

(tested in Chrome, Firefox, and Edge)

Actual behavior:

console output: 43 43

Comments:

The var x; here is the important part. Without it, there is no issue (the expected result is 43 43). Also note that the code will fail to execute with let or const as the variable has already been declared.

Looking at the code TS produces...

(function foo(x, f) {
    if (f === void 0) { f = function () { return x; }; }
    var x;
    x++;
    console.log(x, f());
})(42);

...we can conclude that we can add a IIFE in order to get the expected behavior:

(function foo(x, f) {
    if (f === void 0) { f = function () { return x; }; }
    (function(x, f) {
        var x;
        x++;
        console.log(x, f());
    })(x, f);
})(42);

or

(function foo(x, f) {
    if (f === void 0) { f = (function(x) { return function () { return x; }; })(x); }
    var x;
    x++;
    console.log(x, f());
})(42);

Not sure if something can/should be done or even if it's a known issue.

The same issue can be found in BabelJS.

Also, credits goes to https://www.linkedin.com/groups/121615/121615-6302178339201306628 that made me want to test it in TypeScript.

@jlowcs jlowcs changed the title Scoping issue with closure in default parameters Scoping issue with closure in default parameters when transpiling to ES5 Aug 16, 2017
@RyanCavanaugh
Copy link
Member

Inserting IIFEs seems like massive overkill. Unless someone found this in real code rather than academic exercise, I'm very inclined to Won't-Fix. If anyone ever hit this outside of a constructed example, it seems like the best fix would be "It is illegal (in ES5- targets) to declare a variable with the same name as any parameters if any parameter has a default".

@DanielRosenwasser
Copy link
Member

Looks like a duplicate of #2584.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Aug 25, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Sep 11, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed Sep 11, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants