You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
parameter initializer is bound to the wrong variable #22769
letfoo=1;(function(bar=foo){// unexpected compiler error; works at runtimevarfoo=2;returnbar;// returns 1})();(function(bar=(baz=foo)=>baz){// unexpected compiler error; works at runtimevarfoo=2;returnbar();// returns 1})();(function(bar=foo,foo=2){// correct compiler error, error at runtimereturnbar;})();
Expected behavior:
No error on the first and second function expressions
This used to work in a previous release.
Actual behavior:
Compiler error on the first two function declarations even though the code works at runtime.
Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it.
Variable 'foo' is used before being assigned.
Compiler error on the first two function declarations even though the code works at runtime.
The code as emitted by the TS compiler will not work. the first example for instance will result in bar having the value undefined and not 1.
To generate code that would work, the compiler will have to do the initialization in a function scope other than the function itself, effectively warping the function in an IIFE.
We choose a while back to not do that as it impacts the readability of the generated code, and instead we opted in adding an error instead.
This used to work in a previous release.
I do not think this ever worked. I went back to 1.8 and the same errors are reported.
The code as emitted by the TS compiler will not work. the first example for instance will result in bar having the value undefined and not 1.
That's only true if the target is ES3 or ES5. I'm currently targeting ES2015.
To generate code that would work, the compiler will have to do the initialization in a function scope other than the function itself, effectively warping the function in an IIFE.
Or you could rename all function scoped identifiers (that are used somewhere in parameter initializers) that shadow declarations outside of the function. (cc Ron Buckton (@rbuckton))
You already do similar renames for lexical declarations:
// foo.tsvarfoo=1;{letfoo=2;}// foo.jsvarfoo=1;{varfoo_1=2;}/**********************/// fn.tsletfoo=1;(function(bar=foo){// unexpected compiler error; works at runtimevarfoo=2;returnbar;})();// fn.jsvarfoo=1;(function(bar){if(bar===void0){bar=foo;}varfoo_1=2;returnbar;// returns 1})();
We choose a while back to not do that as it impacts the readability of the generated code, and instead we opted in adding an error instead.
May I ask you to reconsider this decision?
I find this rather concerning as it only applies to code transpiled to ES3 and ES5. It makes using ES2015 features difficult to use because it shows errors that are technically not correct. If I didn't know much ES2015 I would think that it's not possible at all as I assume TypeScript does the correct thing.
You also use this same implementation to power the JS language service which doesn't show errors by default. As a user I'm really confused when "Go to declaration" jumps to a different variable than what's used at runtime.
Or you could rename all function scoped identifiers (that are used somewhere in parameter initializers) that shadow declarations outside of the function.
In the general case, we can't know what other outside variables in scope might exist.
one concern i have is that the semantics of the same code snippet would be different based on your target compilation..
That's why I would still prefer to rename function scoped variables that shadow variables used in a parameter initializer when transpiling to ES3 or ES5. That way you don't need to maintain and test two different implementations depending on the target.
In the general case, we can't know what other outside variables in scope might exist.
You already make similar assumptions when transpiling lexical declarations:
// no-shadow.ts{letfoo=1;}// no-shadow.js{varfoo=1;// assumes there is no variable `foo` in the global scope}/********************/// shadow.tsvarfoo=0;{letfoo=1;}// shadow.jsvarfoo=0;{varfoo_1=1;// assumes there is no variable `foo_1` in the outer scope}
Activity
mhegazy commentedon Mar 26, 2018
The code as emitted by the TS compiler will not work. the first example for instance will result in
barhaving the valueundefinedand not1.To generate code that would work, the compiler will have to do the initialization in a function scope other than the function itself, effectively warping the function in an IIFE.
We choose a while back to not do that as it impacts the readability of the generated code, and instead we opted in adding an error instead.
I do not think this ever worked. I went back to 1.8 and the same errors are reported.
ajafff commentedon Mar 27, 2018
Mohamed Hegazy (@mhegazy)
That's only true if the target is ES3 or ES5. I'm currently targeting ES2015.
Or you could rename all function scoped identifiers (that are used somewhere in parameter initializers) that shadow declarations outside of the function. (cc Ron Buckton (@rbuckton))
You already do similar renames for lexical declarations:
May I ask you to reconsider this decision?
I find this rather concerning as it only applies to code transpiled to ES3 and ES5. It makes using ES2015 features difficult to use because it shows errors that are technically not correct. If I didn't know much ES2015 I would think that it's not possible at all as I assume TypeScript does the correct thing.
You also use this same implementation to power the JS language service which doesn't show errors by default. As a user I'm really confused when "Go to declaration" jumps to a different variable than what's used at runtime.
RyanCavanaugh commentedon Mar 27, 2018
In the general case, we can't know what other outside variables in scope might exist.
mhegazy commentedon Mar 27, 2018
We can disable the whole check if we are in ES2015 or higher.
mhegazy commentedon Mar 27, 2018
feel free to send us a PR for disabling the check for ES2015 or later.
ajafff commentedon Mar 27, 2018
Unfortunately it's not only the error message. The whole name lookup needs to be changed to make this work as expected.
mhegazy commentedon Mar 27, 2018
U are right, we can change that too.
mhegazy commentedon Mar 28, 2018
one concern i have is that the semantics of the same code snippet would be different based on your target compilation..
ajafff commentedon Mar 28, 2018
That's why I would still prefer to rename function scoped variables that shadow variables used in a parameter initializer when transpiling to ES3 or ES5. That way you don't need to maintain and test two different implementations depending on the target.
You already make similar assumptions when transpiling lexical declarations:
RyanCavanaugh commentedon Aug 7, 2018
Accepting PRs to allow the ES6 scoping behavior when targeting ES6+; code should remain an error in downlevel targets