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

Don't try to read imported JSON files #267

Closed
sindresorhus opened this issue Apr 25, 2016 · 8 comments · Fixed by #297 or Urigo/tortilla#68
Closed

Don't try to read imported JSON files #267

sindresorhus opened this issue Apr 25, 2016 · 8 comments · Fixed by #297 or Urigo/tortilla#68

Comments

@sindresorhus
Copy link

In Node.js, this is valid and a common pattern:

var pkg = require('./package.json');

I got errors when I ran this plugin on got:

  test/headers.js:3
  ✖  3:17  Parse errors in imported module '../package.json': Line 2: Unexpected token (2:9)  import/namespace
  ✖  3:17  Parse errors in imported module '../package.json': Line 2: Unexpected token (2:9)  import/no-named-as-default-member
  ✖  3:17  Parse errors in imported module '../package.json': Line 2: Unexpected token (2:9)  import/no-named-as-default
@benmosher
Copy link
Member

You got those errors on a require?

@sindresorhus
Copy link
Author

@benmosher
Copy link
Member

Ah, okay, that makes sense.

I think it probably makes sense to handle this in the Node resolver and return null so the analysis is ignored. (analogous to defining \.json$ as an ignored pattern)

I am assuming you're not expecting it to attempt to lint a json file's imports.

@sindresorhus
Copy link
Author

I am assuming you're not expecting it to attempt to lint a json file's imports.

Correct.

@benmosher
Copy link
Member

Actually, I suddenly think it would make more sense to have a parseable extension whitelist (import/module-extensions setting?) and default to ['js', 'mjs']. That would make #268 really efficient in the case that a project is actually using .mjs everywhere; could set the extensions to just ['mjs'] and it wouldn't even need to read the file and inspect the contents.

Might make sense to also make a rule to flag imports of files that don't match that list, I would want some way to avoid having accidental imports of CoffeeScript or something silently appear valid.

@sindresorhus
Copy link
Author

I like the idea of such option, but should default to ['js'] for now. mjs is far from decided on.

@benmosher
Copy link
Member

Sure. Maybe a better expression of the default would have been ['js', BIKESHEDDED_MODULE_EXT].

@benmosher benmosher mentioned this issue May 2, 2016
2 tasks
@benmosher benmosher removed this from the next milestone May 2, 2016
@benmosher
Copy link
Member

Bumped out of next milestone pending discussion on #297.

benmosher added a commit that referenced this issue May 5, 2016
* import/extensions setting: parser whitelist. fixes #267

* default to all extensions valid to avoid breaking change until semver-next
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants