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

Give better error message when ignoring no-slow-types with deno-lint-ignore #23182

Open
dsherret opened this issue Apr 2, 2024 · 3 comments
Open
Assignees
Labels
bug Something isn't working correctly

Comments

@dsherret
Copy link
Member

dsherret commented Apr 2, 2024

See #22683 (comment)

@dsherret dsherret added the bug Something isn't working correctly label Apr 2, 2024
@dsherret dsherret self-assigned this Apr 2, 2024
@NfNitLoop
Copy link

NfNitLoop commented Apr 16, 2024

I see this context from the other ticket:

It can't be selectively ignored because it's an all or nothing feature. If a single instance of slow types is found in a package then it will not do optimizations to make things faster (which can be significant). So if you could ignore the rule in one place then it's not using the optimizations for the package.

That may be the reason why the no-slow-types rule was added and implemented as it is, but that's not how I experience it as a user.

The no-slow-types check seems to be enabled implicitly on the CLI when the deno.json(c) file has fields that make it look like it's targeting JSR. (name, version, and exports, I think?) So, now my deno lint test fails.

  1. That's actually beneficial in some cases. I found some parts of my public API where I hadn't explicitly declared a return type. I want to have an explicit return type so the intent is documented. (Maybe this could be made a separate lint rule? I'd love to enable this check even when not targeting JSR.)

  2. But in other cases, like when using types generated by Zod, the no-slow-types lint is still triggered. The workaround here is more tedious than I want to take on right now.

I realize I'll need to deal with # 2 before I can publish without --allow-slow-types, but in the meantime I'd like to still get the benefits of # 1, and catch other regressions w.r.t. "slow" (undeclared) return types until I'm prepared to go hand-roll "fast" types for all of my Zod schemas.

So I'd love to be able to ignore the no-slow-types checks for the things I'm not fixing now, so that deno lint will let them pass, but keep me from adding further regressions. deno publish could still disallow ignoring the slow types checks unless --allow-slow-types is enabled.

Additionally, I think it's a mistake to have deno lint fail with what behaves very much like a lint rule, but has exceptions which need extra documentation to call out. Instead, it would better to make it act exactly like a lint rule when operating within the context of deno lint, and add the exceptional behavior (maybe a separate no-slow-types-strict rule, which can't be ignored?) for the exceptional case (deno publish).

@nberlette
Copy link
Contributor

@NfNitLoop in regards to linting for explicit return type annotations, have you tried using the deno doc --lint command? It's still quite new, but it does exactly that. It also alerts you to other inconsistencies in your code's type annotations and JSDoc comments.

@kasper573
Copy link

I find the implication of this very interesting. What does this mean for deno programs in a bigger picture? Heavy inference based typescript libraries like zod, valibot, trpc, drizzle, etc goes all in on these "slow types". Does this mean that using those patterns and libraries are effectively discouraged by deno?

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

No branches or pull requests

4 participants