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

Named exports that are functions can be elevated incorrectly. #73

Closed
rwjblue opened this issue Jan 25, 2015 · 3 comments · Fixed by #75
Closed

Named exports that are functions can be elevated incorrectly. #73

rwjblue opened this issue Jan 25, 2015 · 3 comments · Fixed by #75
Assignees

Comments

@rwjblue
Copy link
Contributor

rwjblue commented Jan 25, 2015

The following input:

var foo = "bar";

if (false) {
  someFunction = function foo() {  };
}

export {
 foo 
}

Results in:

define(['exports'], function (exports) {

  'use strict';

  exports.foo = foo;

  var foo = "bar";

  if (false) {
    var someFunction = function foo() {  };
  }

});

It seems that since a function named foo existed (even though it was not available as foo since it was assigned to someFunction variable) the transpiler assumed that it could hoist the named export to the beginning of the file. In this case, it results in exports.foo being undefined when later required by other modules.

@rwjblue
Copy link
Contributor Author

rwjblue commented Jan 25, 2015

For a less contrived example, see the following code in Ember:

The resulting named export of defineProperty is undefined (since there was no function available but the exports.defineProperty = defineProperty; was hoisted to the beginning of the AMD module).

@rwjblue
Copy link
Contributor Author

rwjblue commented Jan 25, 2015

Another example:

var foo = function foo() { }

export {
 foo 
}
define(['exports'], function (exports) {

    'use strict';

    exports.foo = foo;

    var foo = function foo() { }

});

It is obvious that var foo = function foo() { } doesn't make sense, but it would be nice if the transpilation step didn't cause this failure.

@eventualbuddha eventualbuddha self-assigned this Jan 25, 2015
eventualbuddha added a commit that referenced this issue Jan 25, 2015
When looking for top-level function names we need to only look at nodes that create bindings in the local scope, which are function declarations.

Closes #73.
@Rich-Harris
Copy link
Contributor

I have this fixed locally - am on a plane with crappy WiFi, will push later

On Sun, 25 Jan 2015 13:06 Robert Jackson notifications@github.com wrote:

Another example:

var foo = function foo() { }
export {
foo
}

define(['exports'], function (exports) {

'use strict';

exports.foo = foo;

var foo = function foo() { }

});

It is obvious that var foo = function foo() { } doesn't make sense, but
it would be nice if the transpilation step didn't cause this failure.


Reply to this email directly or view it on GitHub
#73 (comment)
.

eventualbuddha added a commit that referenced this issue Jan 26, 2015
When looking for top-level function names we need to only look at nodes that create bindings in the local scope, which are function declarations.

Closes #73.
eventualbuddha added a commit that referenced this issue Jan 26, 2015
When looking for top-level function names we need to only look at nodes that create bindings in the local scope, which are function declarations.

Closes #73.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants