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

[draft] ES6 import support #57

Closed
wants to merge 2 commits into from
Closed

Conversation

mattdesl
Copy link

Fixes #56 and allows for detective to find import statements.

Note there is also detective-es6 which is probably better suited for common situations, since it doesn't consider require calls.

@zertosh
Copy link
Member

zertosh commented Sep 12, 2015

Cool. A few things: (1) Deciding whether to look for imports based on an option that is already a default passed to acorn doesn't seem fitting. How about something like imports or includeImports? (2) The fast regex check needs to be fixed.

@mattdesl
Copy link
Author

Yeah, good point. I'm also wondering whether this "es6 mode" should not look for require() statements.

Sent from my iPhone

On Sep 12, 2015, at 11:36 AM, Andres Suarez [email protected] wrote:

Cool. A few things: (1) Deciding whether to look for imports based on an option that is already a default passed to acorn doesn't seem fitting. How about something like imports or includeImports? (2) The fast regex check needs to be fixed.


Reply to this email directly or view it on GitHub.

@zertosh
Copy link
Member

zertosh commented Sep 12, 2015

I'm also wondering whether this "es6 mode" should not look for require() statements.

Interesting point. Maybe the opts should be includeImports and onlyImports?

@mattdesl
Copy link
Author

@zertosh I'm taking another stab at this.

How should the word option be affected? Should it optionally support an array like [ 'require', 'import' ]? Or should it just transparently default to both when { includeImports: true, onlyImports: false }?

@mattdesl
Copy link
Author

The PR would be cleaner if opt.word was a regex, but this is a breaking change

@mattdesl
Copy link
Author

Ok, things were starting to get really nasty in this PR, trying to support all the legacy options of detective.

I ended up creating a new module for my needs.
https://www.npmjs.com/package/detect-import-require

@goto-bus-stop
Copy link
Member

I'll close this then in favour of using alternatives like detective-es6 or detect-import-require. thanks!

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

Successfully merging this pull request may close these issues.

3 participants