Skip to content
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

invalid code generated with target "es2015" and "system" modules #6426

Closed
FlorianDold opened this issue Jan 10, 2016 · 3 comments · Fixed by #7559
Closed

invalid code generated with target "es2015" and "system" modules #6426

FlorianDold opened this issue Jan 10, 2016 · 3 comments · Fixed by #7559
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@FlorianDold
Copy link

The following code

export class MyClass { }

export function myFunction() {
    return new MyClass();
}

is compiled (with tsc test.ts -m system --target es2015) to

System.register([], function(exports_1) {
    "use strict";
    var MyClass;
    function myFunction() {
        return new MyClass();
    }
    exports_1("myFunction", myFunction);
    return {
        setters:[],
        execute: function() {
            class MyClass {
            }
            exports_1("MyClass", MyClass);
        }
    }
});

which is wrong, since the actual definition of MyClass is not in scope of myFunction anymore.

Compiling down to es5 works as expected with respect to the scope of the class.

@jkillian
Copy link

Edit: I'm wrong 😄, see #6426 (comment) below

I believe that this is the correct behavior @FlorianDold. When you do export class MyClass { } you are using a class expression instead of a regular class declaration. (Edit: my reasoning here doesn't make any sense, because exporting arbitrary expressions, like export 5, is obviously invalid. There must be an identifier or it must be a default export.) In class expressions, the BindingIdentifier portion (i.e. MyClass) is optional. If provided, as you did, it is only available for use inside of the class body. (See bullet point #2 here).

Instead, you could do something like this:

class MyClass { }
export MyClass;
export function myFunction() {
    return new MyClass();
}

Your original TS code should generate an error, as return new MyClass(); is invalid because MyClass isn't available there. If there's no error, that is a bug most likely.

@weswigham
Copy link
Member

export class MyClass { } is still a class declaration (with an export modifier). Our emit is probably wrong - the declaration within the execute function behaves as block scoped (which is normal), but we need to hoist it so its binding is available in the initialization block above. This is probably best done by rewriting the class as a class expression which is assigned to the var made above. (or leaving it as a declaration, but mangling the hoisted name and assigning it to the mangled name)

@jkillian
Copy link

Thanks for the correction @weswigham. If you did want to export a named class expression in one statement for some reason, assuming you could do something like export default (class SomeName { });?

@mhegazy mhegazy added the Bug A bug in TypeScript label Jan 11, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Jan 11, 2016
@vladima vladima added the Fixed A PR has been merged for this issue label Mar 17, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants