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

[Fix] no-duplicates: Handle TS import type #1676

Merged
merged 1 commit into from
Apr 23, 2020

Conversation

kmui2
Copy link
Contributor

@kmui2 kmui2 commented Mar 4, 2020

Fixes #1667

Don't merge yet!

This PR contains a new test for TypeScript 3.8's new import type syntax. However, TypeScript 3.8 is still not supported yet for TypeScript ESLint! See #1667 (comment).

Updates the following TypeScript packages.

    "@typescript-eslint/parser": "^2.22.0",
    "eslint-import-resolver-typescript": "^2.0.0",
    "typescript": "^3.8.3",

As of this TypeScript ESLint package version, it will continue to produce the following error until TypeScript ESLint gains support for TypeScript 3.8 (progress can be tracked here):

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

Update

TypeScript 3.8 import type is now supported in TypeScript ESLint v2.23.0: https://github.com/typescript-eslint/typescript-eslint/releases/tag/v2.23.0

@coveralls
Copy link

coveralls commented Mar 4, 2020

Coverage Status

Coverage remained the same at 97.808% when pulling 9516152 on kmui2:ts-import-type into caf45a6 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.9%) to 90.881% when pulling 162c0e1 on kmui2:ts-import-type into efd6be1 on benmosher:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.9%) to 90.881% when pulling 162c0e1 on kmui2:ts-import-type into efd6be1 on benmosher:master.

@BPScott
Copy link

BPScott commented Mar 9, 2020

typescript-estree v2.23.0 has been released which contains support for the import/export type format.

It doesn't touch the export * as ns and ECMAScript private fields stuff that was also in TS 3.8 as those AST specs aren't yet finalized but it doesn't look like this PR cares about that functionality

@ljharb
Copy link
Member

ljharb commented Mar 10, 2020

@kmui2 it seems like tests are failing :-/

@kmui2
Copy link
Contributor Author

kmui2 commented Mar 10, 2020

@ljharb I'll upgrade to typescript-eslint 2.23.0 and see if they pass. Seems they added the import type support.

@ljharb
Copy link
Member

ljharb commented Mar 10, 2020

Worth a shot; but ^2.22.0 already includes 2.23.

@kmui2
Copy link
Contributor Author

kmui2 commented Mar 10, 2020

Worth a shot; but ^2.22.0 already includes 2.23.

I think the tests ran before 2.23.0 was released. Anyway, I ran it locally after upgrading and my new import type test seems to pass now. Let's hope the CI tests pass as well.

package.json Outdated Show resolved Hide resolved
@ljharb
Copy link
Member

ljharb commented Mar 10, 2020

Still some failures :-/

@kmui2
Copy link
Contributor Author

kmui2 commented Mar 11, 2020

I'll come back again and fix the tests. They seem to just fail whenever ESLINT_VERSION >= 4.

@ljharb
Copy link
Member

ljharb commented Mar 11, 2020

Let's try to handle import/order as well #1667 (comment)

@kmui2
Copy link
Contributor Author

kmui2 commented Mar 12, 2020

I'm not sure what's going on with ESLINT_VERSION=6 test:

https://travis-ci.org/github/benmosher/eslint-plugin-import/jobs/661380703?utm_medium=notification&utm_source=github_status

I had it passing with ESLint at v6.x.x on my local.

@kmui2
Copy link
Contributor Author

kmui2 commented Mar 16, 2020

I'm really stuck here with the test for Node 13 w/ ESLint version 6. Need to pull in help on this. I am not able to reproduce this.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased this, and it passes, so ¯\_(ツ)_/¯

@ljharb ljharb merged commit 9516152 into import-js:master Apr 23, 2020
@kmui2
Copy link
Contributor Author

kmui2 commented Apr 23, 2020

@ljharb, big thanks on helping finally get this merged! I realize I haven't created a test for the import/order case here. We can do in another PR soon.

@ljharb
Copy link
Member

ljharb commented Apr 23, 2020

Thanks, that'd be great :-)

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

Successfully merging this pull request may close these issues.

"import type" from TypeScript 3.8 not recognized
4 participants