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

type import flagged is missing and unpublished #66

Closed
NullVoxPopuli opened this issue Nov 15, 2022 · 8 comments · Fixed by #139
Closed

type import flagged is missing and unpublished #66

NullVoxPopuli opened this issue Nov 15, 2022 · 8 comments · Fixed by #139

Comments

@NullVoxPopuli
Copy link

idk if there is an existing rule to handle this case, but in Typescript, you can do:

import type { SomeType } from 'some-library';

for context, when transpiling to either CJS or MJS, the import doesn't make it to the output -- obviously a lint plugin can't know that is going to happen, but if there could be some way to relax n/no-missing-import and n/no-unpublished-import, (or require that some-library be in devDependencies if not found in the normal locations, this'd be a huge help <3

@aladdin-add
Copy link

I'm open to that - just wondering what's the best solution on it. the generated .d.ts still requires some-library, so will it break the the lib's users running tsc(if using ts)?

@Systemcluster
Copy link

Systemcluster commented Jul 14, 2023

I'd argue import type should either be ignored for this rule, or it would have to use a different resolver strategy. The second option seems more complicated, since type exports can be either from regular scripts, or .d.ts files in packages that are only referenced by the types field in package.json.

I'm getting this error multiple times here:

image

Note that @octokit/types only exports types. (The first error is a separate issue reported in #75)

@scagood
Copy link

scagood commented Oct 26, 2023

We do have an option for ignoring type imports in no-unpublished-import ignoreTypeImport

Does this cover your use case?

@BrummbaerFr
Copy link

I am running into the exact same problem, using a private package that only exports types.

ignoreTypeImport does fix the problem for no-unpublished-import, however I had to just disable for no-missing-import for the problematic line.

@scagood
Copy link

scagood commented Nov 4, 2023

mmm, could you possibly reproduce the issue in a small repo or in the eslint playground?

@BrummbaerFr
Copy link

BrummbaerFr commented Nov 6, 2023

mmm, could you possibly reproduce the issue in a small repo or in the eslint playground?

Sure! Here it is: eslint-n-missing-import-repro

Since the problematic package in my case is private, I used @octokit/types as pointed out by @Systemcluster.

@scagood
Copy link

scagood commented Nov 6, 2023

That's perfect, thank you so much! 💚

I will try to look in the latter half of this week

@scagood
Copy link

scagood commented Nov 7, 2023

I am thinking about trying to change from import-meta-resolve to enhanced-resolve 🤔

There are a few reasons for this:

  1. You can just use require() to import it, unlike currently where we use a built version of import-meta-resolve
  2. It provides a mechanism to implement the ts paths aliases ( no-missing-import does not work with "paths" from "tsconfig.json" #84)
  3. This allows us to pass extensionAlias into the resolver ([file-extension-in-import] Does not work properly in TypeScript projects with allowImportingTsExtensions #134)
  4. This allows us to handle different file extension ([no-missing-imports]: add tryExtensions option #33)

This could be a little finicky to get right 😅 I will give it a go this weekend to see if its plausible!

Edit

The eslint-import-resolver seems to also use enhanced-resolve

scagood added a commit to scagood/eslint-plugin-n that referenced this issue Nov 25, 2023
scagood added a commit to scagood/eslint-plugin-n that referenced this issue Nov 25, 2023
scagood added a commit to scagood/eslint-plugin-n that referenced this issue Nov 25, 2023
scagood added a commit to scagood/eslint-plugin-n that referenced this issue Dec 11, 2023
scagood added a commit to scagood/eslint-plugin-n that referenced this issue Jan 2, 2024
aladdin-add pushed a commit that referenced this issue Jan 9, 2024
scagood added a commit to scagood/eslint-plugin-n that referenced this issue Jan 9, 2024
aladdin-add added a commit that referenced this issue Jan 9, 2024
* feat: Use enhanced-resolve for imports

* chore: Improve the metadata from "ImportTarget"

* chore: remove "enhanced-resolve" from "no-hide-core-modules"

* test: Add a test for #66

* feat!: Allow ts paths aliases (#84)

* feat: Allow for "allowImportingTsExtensions" (#134)

* feat: Add test for import maps (#147)

* Update lib/util/import-target.js

Co-authored-by: Sebastian Good <2230835+scagood@users.noreply.github.com>

* test: Add test for n/no-missing-require eslint/use-at-your-own-risk

* chore: Remove esbuild

* feat: Allow for settings.cwd to be used before process.cwd

* chore: replace reference to eslint/use-at-your-own-risk

* chore: Remove more unused packages

* chore: update rule test options to flat config

* fix: incorrect env in tests

---------

Co-authored-by: 唯然 <weiran.zsd@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants