Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Inconsistent behaviour with import/ignore #478

Closed
rhys-vdw opened this issue Aug 8, 2016 · 6 comments · Fixed by Urigo/tortilla#68
Closed

Inconsistent behaviour with import/ignore #478

rhys-vdw opened this issue Aug 8, 2016 · 6 comments · Fixed by Urigo/tortilla#68
Assignees
Labels
Milestone

Comments

@rhys-vdw
Copy link

rhys-vdw commented Aug 8, 2016

Getting some strangely inconsistent behaviour with the import/ignore setting.

My config:

// .eslintrc.js
module.exports = {
  "settings": {
    "import/ignore": [
      "\.erb$",
      "\.coffee$",
      "node_modules"
    ],
    "import/resolver": {
      "webpack": {
        "config": "./config/webpack.config.js"
      }
    }
  },
// ... the rest is irrelevant

The erroneous errors:

> eslint --config .eslintrc.js app/assets/javascripts/


/Users/rhys/Projects/usability-hub/usability_hub/app/assets/javascripts/components/test-form/screenshots-editor/screenshots-editor.js
  12:37  error    Parse errors in imported module '../../../views/screenshot-hitzone-editor': Unexpected token %= (8:31)  import/namespace
  12:37  error    Parse errors in imported module '../../../views/screenshot-hitzone-editor': Unexpected token %= (8:31)  import/default
  12:37  warning  Parse errors in imported module '../../../views/screenshot-hitzone-editor': Unexpected token %= (8:31)  import/no-named-as-default
  12:37  warning  Parse errors in imported module '../../../views/screenshot-hitzone-editor': Unexpected token %= (8:31)  import/no-named-as-default-member

The file that that is throwing errors:

Lines 12 and 13 from screenshots-editor.js (the file containing failed imports above):

// ...
import ScreenshotHitzoneEditor from '../../../views/screenshot-hitzone-editor';
import RailsRoutes from '../../../rails-routes';
// ...

The reason why I'm showing both of these lines is because both of these references files are .js.erb files.

Line 8 from screenshot-hitzone-editor.js.erb:

// ...
const SCREENSHOT_MAX_WIDTH = <%= Screenshot::MAX_DISPLAY_WIDTH %>;
// ...

And the same erb syntax used in rails-routes.js.erb:

// ...
<%=
routes = [
  /^choose_test$/,
  /^confirm_destroy_response$/,
// ...

So ESLint will try to parse screenshot-hitzone-editor.js.erb, but will (correctly) avoid parsing rails-routes.js.erb. There are several erb files in the project, and this is the first time that this configuration strategy has failed.

Note that I can replace the import statements with fully qualified paths, and the same problem occurs:

// still fails:
import ScreenshotHitzoneEditor from '../../../views/screenshot-hitzone-editor.js.erb';
import RailsRoutes from '../../../rails-routes.js.erb';

So the problem does appear to be with eslint-import AFAICT.

I'm happy to help debug this.

@rhys-vdw rhys-vdw changed the title Inconsistent behaviour with ignore Inconsistent behaviour with import/ignore Aug 8, 2016
@benmosher
Copy link
Member

There is an unfortunate (but practical) compromise in import/ignore where it will parse ignored paths if they look like they hold exports (based on a regex). It's a fairly grimy bit that I'm very much looking forward to blowing away in v2.

You should be able to remedy this by specifying the import/extensions setting as ['.js'] (or whatever list of extensions you use with ES6 parseable files).

@rhys-vdw
Copy link
Author

rhys-vdw commented Aug 8, 2016

Thanks for the quick response, @benmosher.

You should be able to remedy this by specifying the import/extensions setting as ['.js'] (or whatever list of extensions you use with ES6 parseable files).

  "settings": {
    "import/extensions": [
      ".js",
    ],
    "import/ignore": [
      "\.erb$",
      "\.coffee$",
      "node_modules"
    ],
    "import/resolver": {
      "webpack": {
        "config": "./config/webpack.config.js"
      }
    }
  },

Sadly the above change to my .eslintrc.js did not resolve the issue! (Despite updating to 1.12.0)

it will parse ignored paths if they look like they hold exports (based on a regex)

This has given me a (fragile) workaround for now... I can use module.exports instead of export default.

@benmosher
Copy link
Member

Yeah, it's not great. Sorry about that! I'm surprised you're the first one to hit this in a major way (and report it, anyway).

I'm startled that import/extensions didn't resolve this, though. It should be a hard whitelist, and should completely prevent any non-.js-ending file from being parsed. (and it was published in 1.7.0, looks like, from the changelog).

@benmosher
Copy link
Member

Agh, okay. There is definitely a bug here. import/extensions is also subject to the hasExports regex, which was not my intent. Must have been an oversight that the tests don't cover.

This can be patched in v1, I will try to knock this out some morning this week.

@benmosher benmosher added the bug label Aug 8, 2016
@benmosher benmosher added this to the patch milestone Aug 8, 2016
@benmosher benmosher self-assigned this Aug 8, 2016
@rhys-vdw
Copy link
Author

rhys-vdw commented Aug 8, 2016

@benmosher thanks for the quick and clear response, and, as always, thanks for the fantastic tool. Happy to work around this minor inconvenience. 👍

@benmosher benmosher modified the milestones: 1.13.0, patch Aug 11, 2016
benmosher added a commit that referenced this issue Aug 11, 2016
* always ignore invalid extensions if `import/extensions` is set (fixes #478)

* reboot of npm `watch` script
@benmosher
Copy link
Member

This is also published with 1.13.0! (turned out I needed it too for typescript interop, has the same issue since it has exports)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants