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

Issue with loose mode export { x } from 'x'; #6805

Closed
hzoo opened this Issue Nov 12, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@hzoo
Member

hzoo commented Nov 12, 2017

jaydenseric added a commit to jaydenseric/apollo-upload-client that referenced this issue Nov 13, 2017

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Nov 13, 2017

Member

Marking as a good first issue because this should be a self-contained fix to helper-module-transforms.

This bug is because of a change in semantics that loose mode introduces. With non-loose, we explicitly define all named exports up front so that the names all exist on the exports object before any of the require() calls run. This was fine on non-loose because

Object.defineProperty(exports, 'foo', {
  get(){ return _foo.foo; };
  // ...
});
var _foo = require("foo");

doesn't actually access _foo.foo at declaration time. Since loose mode removes the wrapper, that breaks.

What we probably want for a fix here is to make it

exports.foo = undefined;

var _foo = require("foo");
exports.foo = _foo.foo;

There's similar logic in there already for other types of exports, so hopefully adding logic for this case should be reasonable.

Member

loganfsmyth commented Nov 13, 2017

Marking as a good first issue because this should be a self-contained fix to helper-module-transforms.

This bug is because of a change in semantics that loose mode introduces. With non-loose, we explicitly define all named exports up front so that the names all exist on the exports object before any of the require() calls run. This was fine on non-loose because

Object.defineProperty(exports, 'foo', {
  get(){ return _foo.foo; };
  // ...
});
var _foo = require("foo");

doesn't actually access _foo.foo at declaration time. Since loose mode removes the wrapper, that breaks.

What we probably want for a fix here is to make it

exports.foo = undefined;

var _foo = require("foo");
exports.foo = _foo.foo;

There's similar logic in there already for other types of exports, so hopefully adding logic for this case should be reasonable.

@daft300punk

This comment has been minimized.

Show comment
Hide comment
@daft300punk

daft300punk Nov 13, 2017

Contributor

I'll be working on this.

Contributor

daft300punk commented Nov 13, 2017

I'll be working on this.

@existentialism

This comment has been minimized.

Show comment
Hide comment
@existentialism

existentialism Nov 13, 2017

Member

Derp, my bad for missing this.

@daft300punk thanks! join us in slack if you have any questions!

Member

existentialism commented Nov 13, 2017

Derp, my bad for missing this.

@daft300punk thanks! join us in slack if you have any questions!

@existentialism existentialism added claimed and removed help wanted labels Nov 13, 2017

futagoza added a commit to futagoza/babel-preset-futagozaryuu that referenced this issue Nov 15, 2017

jaydenseric added a commit to jaydenseric/graphql-upload that referenced this issue Nov 18, 2017

Disable Babel loose mode.
We had issues withBabel v7 and `loose` mode and in `apollo-upload-client`, see babel/babel#6805.
@existentialism

This comment has been minimized.

Show comment
Hide comment
@existentialism

existentialism Nov 20, 2017

Member

@daft300punk friendly ping! any updates?

Member

existentialism commented Nov 20, 2017

@daft300punk friendly ping! any updates?

futagoza added a commit to futagoza/babel-preset-futagozaryuu that referenced this issue Dec 12, 2017

@lock lock bot added the outdated label May 3, 2018

@lock lock bot locked as resolved and limited conversation to collaborators May 3, 2018

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