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

Dynamic imports must disallow trailing comma #8957

Open
wchargin opened this Issue Nov 1, 2018 · 3 comments

Comments

Projects
None yet
6 participants
@wchargin

wchargin commented Nov 1, 2018

Bug Report

Current Behavior

Dynamic imports are allowed to have trailing commas. The spec says that
they must not.

Input Code

import("abc");  // OK
import("def", );  // should fail to parse!

BabelJS REPL link.

Expected behavior/code

This should raise a parse error. The ImportCall production has

import ( AssignmentExpression )

and an AssignmentExpression cannot have a trailing comma.

See also: facebook/flow#7124

See also: tc39/proposal-dynamic-import#15

Babel Configuration (.babelrc, package.json, cli command)

Default on REPL (es2015, stage-2, react).

@babel-bot

This comment has been minimized.

Collaborator

babel-bot commented Nov 1, 2018

Hey @wchargin! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@danez

This comment has been minimized.

Member

danez commented Nov 3, 2018

Thanks for bringing this up.
Even though the actual spec currently does not mention a trailing comma, it seems the signal is it might change to be allowed if I interpret @ljharb's comment in tc39/proposal-dynamic-import#15.

So I wonder if we should wait before we work on this.

@ljharb

This comment has been minimized.

ljharb commented Nov 3, 2018

I think it should be allowed; that’s not the same as signaling that it might change. I’d file a new issue in that repo for allowing a trailing comma (or better, make a PR that allows it)

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