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 for Flowtype import/exports #123

Closed
wants to merge 2 commits into from

Conversation

jrehwaldt
Copy link

Add support for Flowtype import/exports (http://flowtype.org/blog/2015/02/18/Import-Types.html) when using babel-eslint parser.

Currently any import type SomeType from './types' is marked as import, but its counterpart export type SomeType is ignored. Therefore, all those imports were marked as issue by rule import/named. This pull request adds the type exports to the list of named exports fixing this issue.

Alternatively, if this behavior is not desired, ImportDeclarations with n.importType === 'type' should be ignored in order to be symmetric. If you prefer this solution give me a hint and I'll create another pull request.

Nice plugin, btw ;-)

…/18/Import-Types.html) when using babel-eslint parser.

Currently any `import type SomeType from './types'` is marked as import, but its counterpart `export type SomeType` is ignored. Therefore, all those imports were marked as issue by rule `import/named`. This pull request adds the type exports to the list of named exports fixing this issue.

Alternatively, if this behavior is not desired, `ImportDeclaration`s with `n.importType === 'type'` should be ignored in order to be symmetric.
@jrehwaldt
Copy link
Author

Hmm. The testing suite/script doesn't seem to be compatible with Windows, so I'd have to make some changes before I can properly test. Give me a hint if you'd be willing to merge and I'll try to fix the test.

Test failed before already. Give me a hint if you'd be willing to merge and need me to make any changes.

@benmosher
Copy link
Member

Ah, cool! I am not using Flow yet, didn't realize it was flagged as a different type.

You're right about the tests: had a different issue from ESLint 1.10 that I just fixed in master.

Could you add some tests? Normally for new syntax I create a new file in the tests/files directory. A test for the import/named rule seems like it would be good. Just something like import type Whatever from './types' in the valid block would be great, with a symmetric import type NotARealType from './types' in the invalid block to ensure it's actually checking.

If not, let me know and I'll try to bang a few out at some point. Not sure when, though, currently trying to get the Webpack resolver shipped this week. 😅

@jrehwaldt
Copy link
Author

I will give it a shot. Do the tests already use babel-eslint parser or do I have to configure that somehow?

@benmosher
Copy link
Member

If you search for "babel-eslint" in the tests folder, you should find some examples.

The

--Ben

On Nov 26, 2015, at 06:32, jrehwaldt notifications@github.com wrote:

I will give it a shot. Do the tests already use babel-eslint parser or do I have to configure that somehow?


Reply to this email directly or view it on GitHub.

@benmosher
Copy link
Member

Sorry, premature send.

The dependency parser is pure Babylon by default; it will need to be configured for Flow by adding the 'flow' plugin. Or you can set import/parser setting to babel-eslint also. The FooES7 tests for no-unresolved have an example of configuring Babylon plugins.

@jrehwaldt
Copy link
Author

I added tests using babel-eslint. Additionally, I tried several more approaches (read: permutations of the code) outlined in the following block resulting in different parse errors either for the export file or the code statement:

test({ code: '/* @flow */ import type { MyType } from "./flowtypes"'
     //, ecmaFeatures: { flow: true }
     //, parser: 'babel-eslint'
     //, settings: { 'import/parse-options': { plugins: [ 'flow' ] } }
}),

Unfortunately, I could not get plugin: 'flow' or any other parser except babel-eslint to work, which is probably due to my lack of understanding of the testing architecture and options.

@jrehwaldt
Copy link
Author

Btw. Flowtype does not support export default ... yet, which is why I added nothing at that end. Not sure if there are additional constructs necessary to support, but I'll guess someone/me will file an issue if something is missed.

@benmosher
Copy link
Member

Ah, yeah, so the plugin test issues were also related to ESLint 1.10 freezing settings. I fixed, added tests, and rebased + committed to master. Thanks for this!

BTW: also added the ability to infer Flow plugin for default Babylon dependency parser via setting ecmaFeatures.flow (#127).

Closing this since it's merged via rebase. Thanks again! 😄

@benmosher benmosher closed this Nov 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants