Skip to content

@types packages are resolved instead of the actual package #265

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

Closed
ahuth opened this issue Jan 12, 2024 · 15 comments
Closed

@types packages are resolved instead of the actual package #265

ahuth opened this issue Jan 12, 2024 · 15 comments

Comments

@ahuth
Copy link

ahuth commented Jan 12, 2024

Possibly the cause of import-js/eslint-plugin-import#2168.

The issue

The resolve function returns the @types/ package associated with a module, if present. This causes issues with lint rules (specifically import/no-extraneous-dependencies).

For example, if we have this scenario:

  1. I don't have a dependency in my package.json on uuid (for example. It also is the same with other deps)
  2. I do have uuid and @types/uuid in my node_modules as a transitive dependencies.
  3. This resolver will return node_modules/@types/uuid as the resolved path for uuid
  4. import/no-extraneous-dependencies will report that I need to install @types/uuid

But that's wrong! Really I need to install uuid

Why this happens

I think this code is resulting in @types packages being resolved when present.

if (
(JS_EXT_PATTERN.test(foundNodePath!) ||
(cachedOptions.alwaysTryTypes && !foundNodePath)) &&
!/^@types[/\\]/.test(source) &&
!path.isAbsolute(source) &&
!source.startsWith('.')
) {
const definitelyTyped = resolve(
'@types' + path.sep + mangleScopedPackage(source),
file,
options,
)
if (definitelyTyped.found) {
return definitelyTyped
}
}

Possible fix

For import/no-extraneous-dependencies, I think @types/ packages should never be resolved, because what we care about is that the actual package is installed, not its types. Presumably tsc will tell us if we don't have the types installed.

However, I'm not sure how the resolved paths are used by other rules.

@JounQin
Copy link
Collaborator

JounQin commented Jan 12, 2024

Have you tried import/external-module-folders setting? Which is enabled in plugin:import/typescript config already.

See also https://github.com/import-js/eslint-plugin-import/blob/7a21f7e10f18c04473faadca94928af6b8e28009/config/typescript.js#L17C1-L17C1

@ahuth
Copy link
Author

ahuth commented Jan 16, 2024

import/external-module-folders doesn't seem to help.

What should the effect of that on resolving @types/foo vs foo be?

@JounQin
Copy link
Collaborator

JounQin commented Jan 16, 2024

This resolver

Prefer resolving @types/* definitions over plain .js/.jsx

import/external-module-folders helps to consider x as installed when @types/x is installed.

If you have even further issue, please provide a minimal but runnable online reproduction instead.

@JounQin JounQin closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2024
@ahuth
Copy link
Author

ahuth commented Jan 16, 2024

import/external-module-folders helps to consider x as installed when @types/x is installed.

That's the opposite of what I'm trying to describe here. Apologies that I didn't explain it clearly.

This issue:

  • x should not be considered installed when it's missing but @types/x is installed.
  • Otherwise, the import/no-extraneous-dependencies rule tells developers to install @types/x, but it should be x (because that's the actual dependency).
  • I think this happens because eslint-import-resolver-typescript always resolves @types/x when present for module x.

Here's a repro: https://stackblitz.com/edit/stackblitz-starters-fshhsu

@JounQin
Copy link
Collaborator

JounQin commented Jan 17, 2024

You didn't enable import/external-module-folders at all?

@ahuth
Copy link
Author

ahuth commented Jan 17, 2024

Tried it and it didn't help (as mentioned in #265 (comment)).

I don't see how it would help. Seems like it does the opposite of what we need to resolve this issue (see #265 (comment)).

@JounQin
Copy link
Collaborator

JounQin commented Jan 17, 2024

Please provide a runnable reproduction then. A git repository.

@JounQin JounQin reopened this Jan 17, 2024
@ahuth
Copy link
Author

ahuth commented Jan 17, 2024

@JounQin #265 (comment) has a link to a runnable reproduction in it.

@JounQin
Copy link
Collaborator

JounQin commented Jan 17, 2024

image

#265 (comment)

I'm really confused.

@ahuth
Copy link
Author

ahuth commented Jan 17, 2024

What are you confused about? I'll try to help make it more clear or work through issues.

index.ts has instructions how to reproduce.

@JounQin
Copy link
Collaborator

JounQin commented Jan 17, 2024

import/external-module-folders setting is not import/no-extraneous-dependencies rule.

@JounQin JounQin closed this as completed Jan 17, 2024
@ahuth
Copy link
Author

ahuth commented Jan 17, 2024

This isn't completed.

  • The no-extraneous-dependencies rule is how I'm reproducing the issue, but isn't the cause.
  • But the issue happens because eslint-import-resolver-typescript is resolving @type/x when it should be resolving x (I think)

@JounQin
Copy link
Collaborator

JounQin commented Jan 17, 2024

Can you set import/external-module-folders first?...

@ahuth
Copy link
Author

ahuth commented Jan 17, 2024

Actually, I was thinking about this wrong. You're right that there's not an issue.

I just realized a couple things:

  • Just because I'm linting typescript doesn't mean I need to use the typescript resolver.
  • The whole point of the typescript resolver is to resolve like typescript does, including "Prefer resolving @types/* definitions" (from the readme)
  • But I can use the node resolver to lint typescript and get the result I want (don't resolve @types/ definitions)

So there's no issue here. Thanks for helping me figure this out.

@JounQin
Copy link
Collaborator

JounQin commented Jan 17, 2024

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

No branches or pull requests

2 participants