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

"import type" from TypeScript 3.8 not recognized #1667

Closed
juergenzimmermann opened this issue Feb 23, 2020 · 10 comments · Fixed by #1676
Closed

"import type" from TypeScript 3.8 not recognized #1667

juergenzimmermann opened this issue Feb 23, 2020 · 10 comments · Fixed by #1676

Comments

@juergenzimmermann
Copy link

After upgrading to TypeScript 3.8.2 I also use the new "import type" feature https://www.typescriptlang.org/docs/handbook/modules.html#importing-types. Now I'm getting a warning from import/no-duplicates.

My affected code is e.g.:

import type { Options } from 'express-rate-limit';
import rateLimit from 'express-rate-limit';
@ljharb
Copy link
Member

ljharb commented Feb 23, 2020

3.8 is brand new, so we definitely need updates to work with it, probably on many rules.

@kmui2
Copy link
Contributor

kmui2 commented Feb 27, 2020

I think this PR may fix this specific issue with type imports: #1666

@noahbrenner
Copy link

Just so we're all on the same page, I thought I'd confirm where we're all coming from. If I understand correctly:

Please correct me if I misunderstood either of you.

@kmui2
Copy link
Contributor

kmui2 commented Mar 4, 2020

Just so we're all on the same page, I thought I'd confirm where we're all coming from. If I understand correctly:

Please correct me if I misunderstood either of you.

Sorry, I was only speculating because I saw the plugin had support for import type coming from Flow types and thought it was something related to a generic default and named export problem.

@noahbrenner
Copy link

No worries! I'm not seeing the string "type" anywhere in that particular PR, could that have just been a typo in the issue number, and there's another PR you found that does cover this? It would be great to see it if so!

@kmui2
Copy link
Contributor

kmui2 commented Mar 4, 2020

No worries! I'm not seeing the string "type" anywhere in that particular PR, could that have just been a typo in the issue number, and there's another PR you found that does cover this? It would be great to see it if so!

I was actually looking into this myself for a bit, but I don't see any other related PR.

@ljharb
Copy link
Member

ljharb commented Mar 4, 2020

#1667 (comment) matches my understanding.

a PR to do that, with tests, would be appreciated.

@kmui2
Copy link
Contributor

kmui2 commented Mar 4, 2020

Seems TypeScript ESLint currently (v2.22.0) does not have support for TypeScript 3.8 yet (>=3.2.1 <3.8.0, typescript-eslint/typescript-eslint#1436). Because of this, it's not currently possible to write a test for the import type syntax. So I'm afraid we need to wait for the TypeScript 3.8 support for ESLint.

For future reference, here's the test I was writing:

context('TypeScript', function() {
  getNonDefaultParsers().forEach((parser) => {
    const parserConfig = {
      parser: parser,
      settings: {
        'import/parsers': { [parser]: ['.ts'] },
        'import/resolver': { 'eslint-import-resolver-typescript': true },
      },
    }

    ruleTester.run('no-duplicates', rule, {
      valid: [
        // #1667: ignore duplicate if is a typescript type import
        test(
          {
            code: "import type { x } from './foo'; import y from './foo'",
            parser,
          },
          parserConfig,
        ),
      ],
      invalid: [],
    })
  })
})
    "@typescript-eslint/parser": "^2.22.0",
    "eslint-import-resolver-typescript": "^2.0.0",
    "typescript": "^3.8.3",

As of these TypeScript ESLint package versions, it will produce the error:

AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: A fatal parsing error occurred in autofix: Parsing error: '{' expected.

@ljharb
Copy link
Member

ljharb commented Mar 4, 2020

Thanks, that makes sense. If you want to open a PR (which would fail until the new version is released) that'd still be appreciated.

@xiaoxiangmoe
Copy link

rule import/order also needs to consider import type and import

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.

5 participants