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

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)

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

Successfully merging a pull request may close this issue.

2 participants