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

This test/src/rules/extensions.js test should be failing with 2 errors not 1 #965

Closed
millerized opened this issue Oct 31, 2017 · 6 comments · Fixed by #1009
Closed

This test/src/rules/extensions.js test should be failing with 2 errors not 1 #965

millerized opened this issue Oct 31, 2017 · 6 comments · Fixed by #1009

Comments

@millerized
Copy link
Contributor

This invalid test should fail with 2 errors – 1 for .js and another for .jsx.

https://github.com/benmosher/eslint-plugin-import/blob/a5844d57be0c3757248c7b824e4cc9c2c844db33/tests/src/rules/extensions.js#L129-L144

The error that I believe is missing:

{
    message: 'Unexpected use of file extension "jsx" for "./bar.jsx"',
    line: 2,
    column: 22,
},
@ljharb
Copy link
Member

ljharb commented Oct 31, 2017

We should probably audit all of the tests for this rule to see where the problem is.

@silvenon
Copy link
Contributor

silvenon commented Feb 1, 2018

I believe this rule behaves correctly because .js is listed as the first extension under settings > import/resolve > extensions, meaning that ./bar will always try to resolve first as ./bar.js, which exists, so it will stop looking further. Regardless of the jsx: 'never' option, adding .jsx should be necessary in order to target bar.jsx specifically, what do you think?

@ljharb
Copy link
Member

ljharb commented Feb 2, 2018

Hmm - if that's true, then a second test, that's an exact duplicate but without the first import line, would fail with just one error - the "missing" one referenced above.

@silvenon
Copy link
Contributor

silvenon commented Feb 2, 2018

Also without the first specified extension being .js. This works:

test({
  code: [
    'import component from "./bar.jsx"',
    'import data from "./bar.json"',
  ].join('\n'),
  options: [ { json: 'always', js: 'never', jsx: 'never' } ],
  settings: { 'import/resolve': { 'extensions': [ '.jsx', '.json', '.js' ] } },
  errors: [
    {
        message: 'Unexpected use of file extension "jsx" for "./bar.jsx"',
        line: 1,
        column: 23,
    },
  ],
}),

@silvenon
Copy link
Contributor

silvenon commented Feb 2, 2018

This might seem like confusing behavior at first, but I see no possible alternative, so forcing the extension if omitting it would not yield the desired result is pretty smart.

@ljharb
Copy link
Member

ljharb commented Feb 2, 2018

Sounds like we might want to add more test cases, but everything's correct as-is?

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

Successfully merging a pull request may close this issue.

3 participants