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

Aliased symbol also exported under non-aliased name. #438

Closed
mbostock opened this issue Jan 8, 2016 · 5 comments
Closed

Aliased symbol also exported under non-aliased name. #438

mbostock opened this issue Jan 8, 2016 · 5 comments

Comments

@mbostock
Copy link
Contributor

mbostock commented Jan 8, 2016

index.js:

export {
  default as axis,
  orientBottom as axisOrientBottom
} from "./src/axis";

src/axis.js:

export var orientBottom = {name: "bottom"};

export default function() {
  var orient;

  function axis() {}

  axis.orient = function() {
    orient = orientBottom;
  };

  return axis;
};

Resulting Rollup with -f amd:

define(['exports'], function (exports) { 'use strict';

  exports.orientBottom = {name: "bottom"};

  function axis() {
    var orient;

    function axis() {}

    axis.orient = function() {
      orient = exports.orientBottom;
    };

    return axis;
  };

  exports.axis = axis;
  exports.axisOrientBottom = exports.orientBottom;

});

Note that the bottom orientation is unexpectedly exported twice: once as orientBottom (wrongly) and once as axisOrientBottom (correctly).

@mbostock
Copy link
Contributor Author

mbostock commented Jan 8, 2016

Here’s an even simpler case for src/axis.js:

export var orientBottom = 42;

export default function() {
  var orient;
  orient = orientBottom;
};

Which produces:

define(['exports'], function (exports) { 'use strict';

  exports.orientBottom = 42;

  function axis() {
    var orient;
    orient = exports.orientBottom;
  };

  exports.axis = axis;
  exports.axisOrientBottom = exports.orientBottom;

});

@mbostock
Copy link
Contributor Author

mbostock commented Jan 8, 2016

I can work around this issue by creating a var instead of an alias in index.js:

import {orientBottom} from "./src/axis";
export {default as axis} from "./src/axis";
export var axisOrientBottom = orientBottom;

I assume this has something to do with exports being bindings rather than constants. Like, it’s protecting against someone importing axisOrientBottom and the setting the value to something else, yeah?

@mbostock
Copy link
Contributor Author

mbostock commented Jan 8, 2016

Another interesting observation is that it produces broken code if the exported symbol is a function. In src/axis.js:

export function orientBottom() {};

export default function() {
  var orient;
  orient = orientBottom;
};

Which produces:

define(['exports'], function (exports) { 'use strict';

  function exports.orientBottom() {};

  function axis() {
    var orient;
    orient = exports.orientBottom;
  };

  exports.axis = axis;
  exports.axisOrientBottom = exports.orientBottom;

});

Which then fails because “exports.orientBottom” isn’t a valid function name.

@mbostock
Copy link
Contributor Author

mbostock commented Jan 8, 2016

Although as I understand it (per 2ality), the imported ES6 binding should be immutable: no one should be able to reassign the axisOrientBottom symbol outside of the module that defines it.

That can’t be strictly enforced by Rollup when converting to ES5 (unless you wanted to protect the exports with a getter/setter rather than using a dumb object, or using Object.freeze, neither of which I’d prefer due to compatibility reasons). But I don’t really care about preventing people from writing broken code. :)

Rich-Harris added a commit that referenced this issue Jan 19, 2016
Rich-Harris added a commit that referenced this issue Jan 19, 2016
Rich-Harris added a commit that referenced this issue Jan 21, 2016
Fix double export of aliased names
@Rich-Harris
Copy link
Contributor

Fixed at last, in 0.25.1 (the first bug, not the function exports.whatever one – tracking that in #454). Thanks

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

No branches or pull requests

3 participants