google / traceur-compiler

Traceur is a JavaScript.next-to-JavaScript-of-today compiler
Apache License 2.0
8.17k stars 580 forks source link

Exclude var __moduleName statement in System.register #2053

Closed guybedford closed 8 years ago

guybedford commented 8 years ago

This removes the unnecessary var __moduleName output from System.register since we have a new wrapping format for this.

There is still a var __moduleName creation in https://github.com/google/traceur-compiler/blob/master/src/codegeneration/ModuleTransformer.js#L132 which can be removed unless this is being used for other formats?

//cc @johnjbarton

@arv I guess you will ask for a test here :) But such a test would be like testing for the non-existence of any arbitrary nonsense scope variable in an output - it shouldn't have been there to begin with.

johnjbarton commented 8 years ago

I'd like to understand why test/node-instantiate-test.js passes. Any ideas?

__moduleName is certainly used in our code, so we'll need to switch to the alternative.

guybedford commented 8 years ago

@johnjbarton it's doing a double __moduleName wrapping, that is it is creating both the variable and the function closure, which both have the same module name value provided. So it's more about the redundancy than anything else.

The change here does not affect __moduleName in the Traceur cases, so we should be good to go? Just let me know if you have any further questions here and thanks for the quick response.

johnjbarton commented 8 years ago

LGTM

Will this change ultimately affect System from es6-module-loader/index ?

arv commented 8 years ago

LGTM

If you wanted to write a test we could check the generated code =P

guybedford commented 8 years ago

Thanks. @arv yes you are right of course!