Skip to content

parameter initializer is bound to the wrong variable #22769

Closed

Description

TypeScript Version: 2.8.0-dev.20180321

Search Terms:

Code

let foo = 1;

(function (bar = foo) { // unexpected compiler error; works at runtime
    var foo = 2;
    return bar; // returns 1
})();

(function (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime
    var foo = 2;
    return bar(); // returns 1
})();

(function (bar = foo, foo = 2) { // correct compiler error, error at runtime
    return bar;
})();

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.

Playground Link:

Related Issues:

Activity

  1. mhegazy commented on Mar 26, 2018

    @mhegazy
    Contributor

    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.

  2. ajafff commented on Mar 27, 2018

    @ajafff
    ContributorAuthor

    Mohamed Hegazy (@mhegazy)

    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.ts
    var foo = 1;
    {
        let foo = 2;
    }
    
    // foo.js
    var foo = 1;
    {
        var foo_1 = 2;
    }
    
    /**********************/
    
    // fn.ts
    let foo = 1;
    
    (function (bar = foo) { // unexpected compiler error; works at runtime
        var foo = 2;
        return bar;
    })();
    
    // fn.js
    var foo = 1;
    (function (bar) {
        if (bar === void 0) { bar = foo; }
        var foo_1 = 2;
        return bar; // 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.

  3. RyanCavanaugh commented on Mar 27, 2018

    @RyanCavanaugh
    Member

    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.

  4. mhegazy commented on Mar 27, 2018

    @mhegazy
    Contributor

    That's only true if the target is ES3 or ES5. I'm currently targeting ES2015.

    We can disable the whole check if we are in ES2015 or higher.

  5. mhegazy commented on Mar 27, 2018

    @mhegazy
    Contributor

    feel free to send us a PR for disabling the check for ES2015 or later.

  6. ajafff commented on Mar 27, 2018

    @ajafff
    ContributorAuthor

    Unfortunately it's not only the error message. The whole name lookup needs to be changed to make this work as expected.

  7. mhegazy commented on Mar 27, 2018

    @mhegazy
    Contributor

    U are right, we can change that too.

  8. added
    SuggestionAn idea for TypeScript
    and removed
    Working as IntendedThe behavior described is the intended behavior; this is not a bug
    on Mar 28, 2018
  9. mhegazy commented on Mar 28, 2018

    @mhegazy
    Contributor

    one concern i have is that the semantics of the same code snippet would be different based on your target compilation..

  10. ajafff commented on Mar 28, 2018

    @ajafff
    ContributorAuthor

    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
    {
        let foo = 1;
    }
    
    // no-shadow.js
    {
        var foo = 1; // assumes there is no variable `foo` in the global scope
    }
    
    /********************/
    
    // shadow.ts
    var foo = 0;
    {
        let foo = 1;
    }
    
    // shadow.js
    var foo = 0;
    {
        var foo_1 = 1; // assumes there is no variable `foo_1` in the outer scope
    }
  11. RyanCavanaugh commented on Aug 7, 2018

    @RyanCavanaugh
    Member

    Accepting PRs to allow the ES6 scoping behavior when targeting ES6+; code should remain an error in downlevel targets

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Effort: ModerateRequires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual".FixedA PR has been merged for this issueHelp WantedYou can do thisSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions