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

resolve .ts/.tsx/.d.ts first, and then fallback to @types/* #20

Merged
merged 6 commits into from Sep 27, 2019

Conversation

@JounQin
Copy link
Collaborator

JounQin commented Jun 30, 2019

No description provided.

@JounQin JounQin force-pushed the rx-ts:feat/resolve_dts branch from 1e73cce to 9a4c30a Jun 30, 2019
@JounQin JounQin force-pushed the rx-ts:feat/resolve_dts branch from 9a4c30a to b11ede3 Jun 30, 2019
@JounQin JounQin changed the title feat: resolve .d.ts path automatically for .jsx? files resolve .ts/.tsx/.d.ts first, and then fallback to @types/* Jun 30, 2019
@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Jul 5, 2019

@alexgorbatchev @bradzacher Do you have any time to review this PR? It is really needed.

@rdsedmundo

This comment has been minimized.

Copy link

rdsedmundo commented Jul 23, 2019

+1 for reviewing, really looking forward to having this.

@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Jul 23, 2019

@rdsedmundo You can try yarn add -D eslint-import-resolver-typescript@JounQin/eslint-import-resolver-typescript#feat/resolve_dts temporarily.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@trevorblades

This comment has been minimized.

Copy link

trevorblades commented Jul 25, 2019

Works like a charm with the alwaysFindTypes option. Thanks @JounQin! 👍

@Protectator

This comment has been minimized.

Copy link

Protectator commented Sep 10, 2019

Any news on a potential merge ?

@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Sep 10, 2019

Hey guys, it seems @alexgorbatchev has 'disappeared' for a few months, or he just forgot this project, so I'm going to fork this repository and republish it as eslint-import-resolver-ts to npm. You can simply change your eslint config:

'import/resolver': {
- typescript: {
+ ts: {
    alwaysTryTypes: true,
  },
},

If anyone is interested, you can follow eslint-import-resolver-ts, PR welcome.

And I will not close this PR, in case @alexgorbatchev return.

@Protectator

This comment has been minimized.

Copy link

Protectator commented Sep 10, 2019

Great, thanks for taking the initiative.

@ljharb

This comment has been minimized.

Copy link

ljharb commented Sep 10, 2019

@JounQin if it's not too late, it's best to publish forks under a scope.

@trevorblades

This comment has been minimized.

Copy link

trevorblades commented Sep 10, 2019

@ljharb @JounQin you should be able to unpublish NPM packages within 48 hours of them being published. Would the scoped name you're referring to be something like @jounqin/eslint-import-resolver-typescript?

@ljharb

This comment has been minimized.

Copy link

ljharb commented Sep 10, 2019

Yes, exactly.

fregante referenced this pull request in xojs/xo Sep 10, 2019
@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Sep 10, 2019

@ljharb scoped package shorthand is supported by eslint-plugin-import?

{
  "@rxts/typescript": {}
}

It's a bit long to me.


It tried @rxts/typescript, it seems it's not supported:

Resolve error: unable to load resolver "@rxts/typescript"

What means if we change to a scoped package, we have to use:

{
  "@rxts/eslint-import-resolver-typescript": {}
}

Seems bad to me.

Of course, if eslint-plugin-import supports short hand for scoped resolver, I will republish it a scoped package @rxts/typescript.

@ljharb

This comment has been minimized.

Copy link

ljharb commented Sep 11, 2019

i don’t believe we support a shorthand, but the priority imo shouldn’t be “what you have to copy and paste into your config file”, but rather “what will make it very clear this is a fork”

@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Sep 11, 2019

@ljharb @trevorblades I'm not only going to fork it without other changes but also refactor, add new features and try to maintain it in the future.

Of course, if @alexgorbatchev return and accept, I can PR back.

Please help to review rx-ts#10 if you don't mind.

@alexgorbatchev alexgorbatchev merged commit 6bb231e into alexgorbatchev:master Sep 27, 2019
@alexgorbatchev

This comment has been minimized.

Copy link
Owner

alexgorbatchev commented Sep 27, 2019

Sorry for MIA :( life...

@JounQin JounQin deleted the rx-ts:feat/resolve_dts branch Sep 27, 2019
@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Sep 27, 2019

Welcome back @alexgorbatchev . So are you going to maintain this resolver in the future? I can PR back from my fork https://github.com/rx-ts/eslint-import-resolver-ts which has been refactored to be .ts based itself with a few new improvements, maybe.

@alexgorbatchev

This comment has been minimized.

Copy link
Owner

alexgorbatchev commented Sep 27, 2019

I think it would make sense to merge back. I can make you a collaborator on the repo. Would that work?

@JounQin

This comment has been minimized.

Copy link
Collaborator Author

JounQin commented Sep 28, 2019

@alexgorbatchev That would be great.

@alexgorbatchev

This comment has been minimized.

Copy link
Owner

alexgorbatchev commented Sep 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.