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

🐛 Strict mode's "Type-only imports should be in devDependencies" is incorrect #951

Closed
6 tasks done
fregante opened this issue Feb 20, 2025 · 4 comments
Closed
6 tasks done
Labels
bug Something isn't working

Comments

@fregante
Copy link

fregante commented Feb 20, 2025

Prerequisites

Reproduction url

Technically not a "bug"

Reproduction access

  • I've made sure the reproduction is publicly accessible

Description of the issue

While it's true that local types should not be part of dependencies, this however is not accurate when the imported type becomes part of the package's public API.

Example:

import type { ErrorObject } from "serialize-error";

export type Soup = {error: ErrorObject} | {ingredients: string[]}

If a third party installs my package, Soup will be of type any. The fix is to install serialize-error separately — which means serialize-error is indeed a required dependency of my package, not a dev dependency.

@fregante fregante added the bug Something isn't working label Feb 20, 2025
@webpro
Copy link
Member

webpro commented Feb 21, 2025

While I appreciate your continued and creative ways to not provide an actual reproduction, this report is a duplicate of #248.

@webpro webpro closed this as completed Feb 21, 2025
@fregante
Copy link
Author

fregante commented Feb 21, 2025

creative ways to not provide an actual reproduction

?

What's the point of a repro if you're already aware of the behavior? 😅

The repro is just this anyway:

npm init -y
npm install serialize-error
echo "$(pbpaste)" > index.ts # copy my example code first
knip --strict

this report is a duplicate of #248

That issue only talks about @types packages, this does not (and even then, that issue raises good concerns)

All in all, following this advice leads to publishing broken/any packages, particularly because it's hard to catch this issue until you've run npm publish (or npm pack + npm install + run tests on the output)

Even TypeScript maintainers disagree with the advice: microsoft/types-publisher#81 (comment)

@webpro
Copy link
Member

webpro commented Feb 23, 2025

creative ways to not provide an actual reproduction

?

What's the point of a repro if you're already aware of the behavior? 😅

The repro is just this anyway:

npm init -y
npm install serialize-error
echo "$(pbpaste)" > index.ts # copy my example code first
knip --strict

I don't want to get into this discussion, all I ask for is to respect my time. Chances to get help simply increase if I don't have to make assumptions and I don't have to create minimal reproductions and fixtures myself.

this report is a duplicate of #248

That issue only talks about @types packages, this does not (and even then, that issue raises good concerns)

All in all, following this advice leads to publishing broken/any packages, particularly because it's hard to catch this issue until you've run npm publish (or npm pack + npm install + run tests on the output)

Even TypeScript maintainers disagree with the advice: microsoft/types-publisher#81 (comment)

I don't disagree with the issue, it's just that I'm still in the same boat (quoting myself):

For this particular issue, the cost/impact does not warrant the outcome (for me; feel tree to take a stab at it)

You can use ignoreDependencies for such exceptions. I have also updated the docs here: https://knip.dev/guides/handling-issues#type-definition-packages

@fregante
Copy link
Author

I don't want to get into this discussion, all I ask for is to respect my time

My point is that repro was not needed here because you're aware of the issue/behavior, you wouldn't even have checked it out, pointing to a waste of my time.

I can always provide a repro if necessary, and I did in my other issues. "Repro please" would have sufficed without getting creative.

I myself am an OS maintainer and feel bad insta-closing issues where people clearly put way too much effort into opening, just because they overlooked something simple or did not find similar requests.

I have also updated the docs here: knip.dev/guides/handling-issues#type-definition-packages

Thank you! Appreciated the effort you put into the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants