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

support protected identifiers #22

Closed
flying-sheep opened this issue Dec 21, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@flying-sheep
Copy link

commented Dec 21, 2015

as it’s perfectly reasonable to do exports.default = foo, yet the transformed syntax is the illegal export var default = connect.default

probably coincides with #10, as the project in question was react-redux, which seems to be compiled with babel

@flying-sheep

This comment has been minimized.

Copy link
Author

commented Dec 21, 2015

another problem is that apparently acorn doesn’t handle comments in function declarations or so…

function compileMap(/* lists... */) {

raises unexpected in parseExprAtom. both were found via this hack: rollup/rollup#296 (comment)

isn’t there better parsers than acorn? e.g. babylon? i guess the first comment is wrong code generation, but this comment is probably a parser not coded to spec…

@Swatinem

This comment has been minimized.

Copy link
Member

commented Dec 22, 2015

I’m also stumbled across this just now. The export var default = X.default trips up the rollup parser.

@flying-sheep

This comment has been minimized.

Copy link
Author

commented Dec 22, 2015

that should be handled in a special way anyway, since babel AFAIK started to translate it that way (with an explicit member “exports.default = ...“ instead of directly as “module.exports = ...”

@Victorystick Victorystick added the bug label Dec 22, 2015

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2015

#16 fixes this – I've updated that branch in light of some more recent changes. @Victorystick if you get a chance to review it, should be good to merge?

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2015

@flying-sheep that's... very odd. Acorn should parse that comment just fine. Are you able to reproduce that specific bug separately?

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 28, 2015

I've released 2.1.0 with the fix. @flying-sheep I don't know if that parser error deserves a separate issue or if rollup/rollup#296 (comment) covers it? Either way I'll close this as it's unrelated to the reserved identifiers thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.