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

Typescript import/named with types in d.ts #1341

Closed
ThomasdenH opened this issue Apr 23, 2019 · 31 comments
Closed

Typescript import/named with types in d.ts #1341

ThomasdenH opened this issue Apr 23, 2019 · 31 comments

Comments

@ThomasdenH
Copy link

I think I have found a bug with import/named. I have created a repository that reproduces the issue.

The line that gives problems is

import { AreaProps } from 'recharts';

It imports an interface defined in a d.ts file. The following warning is given:

1:10  error  AreaProps not found in 'recharts'  import/named

PR #1304 should solve importing types to typescript, but maybe this doesn't work for declaration files? Or is there something wrong with my configuration?

@manuelbieh
Copy link

manuelbieh commented Apr 30, 2019

Struggling with a similar issue and express-serve-static-core from node_modules/@types/express-serve-static-core. I don't get an import/named error but an import/no-unresolved error. Looks like .d.ts files aren't resolved properly

import { RequestHandler, Request } from 'express-serve-static-core';

results in an error:

8:41  error  Unable to resolve path to module 'express-serve-static-core'  import/no-unresolved

// edit:
looks like eslint still uses eslint-import-resolver-node instead of eslint-import-resolver-typescript:

eslint-plugin-import:resolver:node resolve threw error: { 
  Error: Cannot find module 'express-serve-static-core' from '.\src\server\middleware'
}

Minimal reproducible example here:
https://github.com/manuelbieh/eslint-typescript-import-bug

@pcorpet
Copy link
Contributor

pcorpet commented May 1, 2019

The problem seems to come for types installed through http://definitelytyped.org/.

@benmosher
Copy link
Member

benmosher commented May 1, 2019

@ThomasdenH can you try using https://www.npmjs.com/package/eslint-import-resolver-typescript as your resolver and see if it fixes?

if it does, I think it may solve many TS-related issues. I just used it to solve some weird ones in my day job codebase

edit: I think it won't help after all, based on import-js/eslint-import-resolver-typescript#17 + associated PR but it's an easy experiment

@benmosher
Copy link
Member

@manuelbieh I think your case is a little different than OP. I see you're using the typescript resolver but it works fine for me... and no-unresolved should report based on the existence of the true code file and not the types.

it would be possible for the typescript resolver to try a @types/... resolution but that would be in scope for that resolver, and not this plugin.

@pcorpet
Copy link
Contributor

pcorpet commented May 5, 2019

@benmosher , thanks the typescript resolver worked for me. Note for reference that you can have multiple import resolvers (I didn't want to turn off the webpack resolver so I hadn't tried the typescript one earlier).

@ThomasdenH
Copy link
Author

ThomasdenH commented May 6, 2019

@ThomasdenH can you try using https://www.npmjs.com/package/eslint-import-resolver-typescript as your resolver and see if it fixes?

if it does, I think it may solve many TS-related issues. I just used it to solve some weird ones in my day job codebase

edit: I think it won't help after all, based on alexgorbatchev/eslint-import-resolver-typescript#17 + associated PR but it's an easy experiment

Sorry for the late reply! Unfortunately it doesn't fix the issue, but thanks for the suggestion.

@benmosher
Copy link
Member

@ThomasdenH: yep, makes sense. I didn't fully understand the scenario when I originally suggested.
@pcorpet: great! I'm doing the same thing -- I have the webpack resolver first for most things but the typescript resolver does resolution in some of my untranspiled first-party TS dependencies

@joelle-helgert-wittgruppe

I am having the same issue for an intl-type that is generated by DefinitelyTyped. Is there any solution or further ideas on how to fix it?

@JounQin
Copy link
Collaborator

JounQin commented Jun 30, 2019

For anyone maybe interested, you can try eslint-import-resolver-typescript@JounQin/eslint-import-resolver-typescript#feat/resolve_dts temporarily (PR made at import-js/eslint-import-resolver-typescript#20).

@benmosher
Copy link
Member

☝️ @JounQin's solution is my recommendation as the permafix. I haven't tried it myself but I scanned through the changes and they look like they should do the trick!

@JounQin: I'm not sure how to read that link. Did you publish your fork to npm?

@JounQin
Copy link
Collaborator

JounQin commented Jul 1, 2019

@benmosher It's just using npm package with github branch.

@dontsave
Copy link

I'm running into this issue as well. Imports from declarations are very common, so this is a real pain. It would be great to get a fix

@benmosher
Copy link
Member

again, resolution is out of scope for this plugin. will close when @JounQin's PR is accepted or other similar fix is implemented by eslint-import-resolver-typescript (see #1341 (comment) above)

@hedgepigdaniel
Copy link

The way I understand it, its not possible for a resolver to solve this problem. Resolvers can only return a single file, but in this case of separate source and declaration files, the exports are distributed across 2 files.

e.g.

// file.d.ts
export type Fruit = {
  color: string,
};
// file.js
export const tomato = {
  color: 'red',
};
// index.ts
import { Fruit, tomato } from './file'; // <-- these exports do not come out of any one file

@JounQin
Copy link
Collaborator

JounQin commented Sep 16, 2019

@hedgepigdaniel You should add export const tomato: Fruit into .d.ts at the same time.

@JounQin
Copy link
Collaborator

JounQin commented Sep 16, 2019

@benmosher Should this issue be closed in favor of eslint-import-resolver-ts?

@w4-alibek
Copy link

w4-alibek commented Dec 11, 2019

Make sure you have inside the
package.json

{
  ....
  "dependencies": {
    "module-name": "version"
  },
  "devDependencies": {
    ...
    "@types/module-name": "version" // version should be same with dependencies
  }
  ....
}

@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@benfletcher @ThomasdenH @ljharb @stekycz

This issue can be closed definitely in favor of eslint-import-resolver-typescript@2.0.0.

@hedgepigdaniel
Copy link

@JounQin which change in that new version addresses this issue?

@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@hedgepigdaniel You can always read the CHANGELOG by yourself.

@ljharb
Copy link
Member

ljharb commented Dec 11, 2019

@JounQin i've done so, and i have no idea which change addresses this issue. Could you clarify?

@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@ljharb I was thinking we were in the same context here: import-js/eslint-import-resolver-typescript#20 (comment), didn't we?

And also #1341 (comment) and #1341 (comment)

@ljharb
Copy link
Member

ljharb commented Dec 11, 2019

Yes but those don't tell me which specific commits in the changelog address this issue :-)

@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@ljharb Did you read https://github.com/alexgorbatchev/eslint-import-resolver-typescript/blob/master/CHANGELOG.md#features?

  • resolve .ts/.tsx/.d.ts first, and then fallback to @types/* (b11ede3)

I made that PR first and accidentally found it could fix this issue already. I don't think that's my responsibility to clarify that this issue has been fixed by another third party package which has been released.

@hedgepigdaniel
Copy link

Does that mean there is now a problem where if there is a declaration file, there is a false error when importing a value from the module that has the declarations?

@hedgepigdaniel
Copy link

I guess that isn't a problem if you have types for all exported symbols

@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@hedgepigdaniel Sorry my English is not so good enough for understanding what's your point? Do you have an example?


Maybe I can understand what's your meaning now.

Of course, .d.ts should always contain all exported symbols from .js.

@hedgepigdaniel
Copy link

// apple.js

export const color = 'red';
export const shape = 'round';
// apple.d.ts

export const color: string;
// index.ts

// vv Error: shape is not defined in `apple.d.ts`!
import { shape } from './apple';

@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@hedgepigdaniel Yeah, but that is a bad practice and should never happen.

@ljharb
Copy link
Member

ljharb commented Dec 11, 2019

@JounQin it's not about what's your responsibility; it's about being friendly and helpful :-) Thanks for pointing me to import-js/eslint-import-resolver-typescript@b11ede3.

@ljharb ljharb closed this as completed Dec 11, 2019
@JounQin
Copy link
Collaborator

JounQin commented Dec 11, 2019

@JounQin it's not about what's your responsibility; it's about being friendly and helpful :-) Thanks for pointing me to alexgorbatchev/eslint-import-resolver-typescript@b11ede3.

@ljharb Emm... I'm always friendly, but my pointed CHANGELOG and PR both target the same commit which fixed this issue. Maybe we both need to be more patient and find out the key point by ourselves? Cheers.

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

No branches or pull requests

10 participants