Skip to content
This repository has been archived by the owner on May 19, 2018. It is now read-only.

Disallow import type { type a } from '' #305

Merged
merged 1 commit into from
Jan 17, 2017
Merged

Disallow import type { type a } from '' #305

merged 1 commit into from
Jan 17, 2017

Conversation

danez
Copy link
Member

@danez danez commented Jan 17, 2017

Q A
Bug fix? yes
Breaking change? no
New feature? no
Deprecations? no
Spec compliancy? yes
Tests added/pass? yes
Fixed tickets
License MIT

I saw that flow was failing, but babylon parsed it just fine.

import typeof {typeof t} from "foo";
import type {type t} from "foo";

@jeffmo Can you confirm that this is correct?

@codecov-io
Copy link

codecov-io commented Jan 17, 2017

Current coverage is 97.27% (diff: 100%)

Merging #305 into master will increase coverage by <.01%

@@             master       #305   diff @@
==========================================
  Files            21         21          
  Lines          3999       4000     +1   
  Methods         489        469    -20   
  Messages          0          0          
  Branches       1169       1191    +22   
==========================================
+ Hits           3890       3891     +1   
  Misses           49         49          
  Partials         60         60          

Powered by Codecov. Last update 28c467e...fc069fd

@jeffmo
Copy link
Contributor

jeffmo commented Jan 17, 2017

Yes this invariant is correct, I must've overlooked it. Thanks for the follow-up!

@danez danez merged commit 999b655 into master Jan 17, 2017
@danez danez deleted the fix-import-short branch January 17, 2017 19:34
@@ -0,0 +1,3 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm probably just not familiar with how the test runner/fixtures work, but shouldn't this output have 2 throws (one for each line)?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, babylon throws at the first error, and never parses the second line. I extracted that in a second test.

@@ -1223,6 +1223,10 @@ export default function (instance) {
specifier.local = specifier.imported.__clone();
}

if (node.importKind !== "value" && specifier.importKind !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Per this discussion with @loganfsmyth, I guess this should stick to explicitly listing out "type" and "typeof"

babel/babel#5035 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed that in #312

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

Successfully merging this pull request may close these issues.

3 participants