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

Allow simultaneous named and default imports #600

Closed
wants to merge 1 commit into from

Conversation

jeffmo
Copy link
Contributor

@jeffmo jeffmo commented Jul 4, 2015

Previously Flow considered import Foo from ... and import Foo, {Bar, Baz} from ... to be mutually exclusive -- though they should not be (per spec).

This fixes Flow to properly handle import Foo, {Bar, Baz} from ...

| None ->
failwith (
"Parser error: Non-default imports must have a set of " ^
"named import specifiers!"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible to import for side effects only, without specifying any bindings, default or non? See #574 for an issue, and the spec at 15.2.2; import ModuleSpecifier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yea, forgot about that issue. Will fix now

@samwgoldman
Copy link
Member

Cool! This is a simple enough diff, and clearly does what it says on the tin. For tracking purposes, this fixes #525.

@nmn
Copy link
Contributor

nmn commented Jul 7, 2015

Just want to report in saying that I merged this temporarily on my local repo and it works as intended for me. Will run the full test suite and report back.

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

Successfully merging this pull request may close these issues.

5 participants