-
Notifications
You must be signed in to change notification settings - Fork 126
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
"Slow types" feel opinionated #444
Comments
Same issue #447 |
Hey! JSR uses type information for multiple features, for example to be able to emit type declarations and generate documentation. To do this, we need to look at the types of any exported symbols (functions, classes, public class properties, public class methods, class constructors, const bindings, let bindings, and var bindings). Usually, looking at type information requires full type checking. Full type checking is broadly four things:
There are a couple of problems with full type checking though. The only tool that implements this right now is the TypeScript compiler ( Firstly, Secondly, Thirdly, as I mentioned earlier, type checking with inference is not just affected by code and configuration the current package, but also by all dependent packages. This is a problem, because dependent packages can change over time. You could publish a new version of a package that changes the types of exported symbols in a way that results in a different inferred type in your local package code. Thus if we check your code with a current dependency version, it may not match a type check we perform in a month with a newer dependency version. With this in mind, JSR encourages projects to not use complex type inference ("slow types") for any exported symbols. Because this requires that you specify explicit type annotations in the source code, it allows us to get all type information needed for declaration emits and documentation generation purely from the syntax of the code, without having to perform any part of type checking or even resolving of dependencies. No This is a direction TypeScript is going separately from JSR via the new So to conclude: we do not enforce type annotations "just for fun", or because we think it's "just the right thing to do". We do it to overcome technical issues posed by the current state of the TypeScript ecosystem. If the conditions change in the future, the code that JSR allows may change - but for now, explicit type annotations are needed to ensure a stable declaration output and documentation generation. Happy to answer any other questions you may have :) |
Gotcha @lucacasonato and thanks for the detailed explanation. With that context in mind, I feel okay with adding explicit types to our generated functions at https://github.com/hey-api/openapi-ts. However, relying on people to tell us that our generated code isn't compliant with JSR won't be ideal. Is there any tooling we can use to make sure our generated code is good to go for people who use it in their packages that are being published to JSR? |
Thanks Luca. That makes sense. A few thoughts:
JSR can mention "generated with tsc 5.2" (i) "may not be relevant with other versions". And / or re-generate declarations once per X months.
Can this process be done client-side? An optional flag like As for re-using dependency types - yeah, I don't see how this could be solved for packages which use version ranges. However, for packages which set versions to an exact value, that's easy. Unless dependencies of dependencies also use version ranges. |
@mrlubos Yes, you can use either @paulmillr You may be interested in #15. You could run this before publishing in CI, and in most cases it will be able to fix all the slow types 👍 |
@lucacasonato proper direction. |
I want to chime in as well to say that "slow types" feels opinionated, albeit from a different perspective. As a library developer, I prefer writing JavaScript and document types using JSDoc to get most of the same benefits of writing in TypeScript without a compilation step. Fwiw, Svelte/Kit prefers the same approach for libraries. My interest in publishing to JSR instead of NPM is to foster ESM more than the use of TypeScript. So having the score reduced because I prefer writing in JavaScript seems a limiting experience and makes me want to reevaluate my choice on where to publish. |
@vijay8i You're score is reduced for not having types, not for writing JavaScript. If you publish dts files for your project, you will get the points related to slow types. |
@lucacasonato I am aware of numerous attempts by other people to “just rewrite tsc in Rust” and they all have failed in one way or another. However, the general case you are describing is not the common case. Most functions and symbols exported by libraries have rather simple types that could be inferred easily by some Rust-based analysis, i.e. a tiny subset of tsc that works well for the common case but falls back to reporting slow types in the complex case. Do you think it is worth writing a tool that can infer some return types in some easy cases, so that JSR can use it with a best-effort approach? |
@lucacasonato Thanks. Good to know. Looks like TypeScript can generate .d.ts from .js files. It would improve the DX of publishing to JSR if it could figure out how to do the invocation automatically or through some directive in either Also apologies to @paulmillr, it is not my itention to sidejack the original issue. Happy to open a new issue if its worth creating one. |
@KnorpelSenf It is the aim of slow types to report diagnostics only for cases that can not be analyzed by looking at the source code of the current module. For example, we can infer: Also see https://jsr.io/docs/about-slow-types#simple-inference @vijay8i We can not invoke If you want to emit .d.ts files from your .js + tsdoc sources, you will have to do this locally using your own tsconfig.json file. We are tracking being able to emit some JS + tsdoc server side without TSC in #494. |
@lucacasonato #494 is fire. Wish I had the Deno chops to get it done. So looking forward to it getting implemented soon(ish?). Thanks! |
May I ask how do you do that? I have tried to find an example in the docs but there is none. I have tried to use I understand that the docs are stating:
But I believe there should still be an example of how to do this with the pair of files. |
Same question. I did the following and I still see slow types warning in the score. "publish": {
"include": [
"README.md",
"package.json",
"src/**/*.?s"
]
} There are a few more annoyances. I don't find it impractical to maintain information in two different files ( |
Right now the |
@marvinhagemeister that worked pretty well. Thanks a lot! |
+1 on renaming this score to "opaque definition", "untyped export", "un-inferable item" etc |
Hi, I have publish dts files in my package, but still got nil score in slow type. Thanks |
I'm ready to leave the deno ecosystem because of economic choices like this. The hours of time it took me to change every module to comply with the requirement is worth more than 1 minute of compute time, even if recomputes need to happen on rebuilds. I really want to believe in the promise of deno's simplicity, and this really betrays that promise. (Also DX issues like the amount of time lost trying to figure out issues like why |
There are scenarios where the "slow types" requirement/error is far too onerous. For example, requiring an explicit Personally I feel the whole "slow types" restriction is just JSR shifting it's own problem onto the user. Reducing type inference doesn't necessarily improve the quality of code. I'd argue "over-typing" is superfluous and visual noise that can make code harder to read. For that reason, I think it should be opt-in by default rather than opt-out. Allow users to opt-in for a better score or shiny badge. Having "slow types" on by default can only discourage users publishing to JSR. Seeing the "slow types" error rejection suggests the code is wrong and has errors, which is not true. It's just negativity. Make it opt-in and show a helpful suggestion rather than an "error". |
Instead of server side, why not run |
That's an interesting idea but it would lock users further into JSR tooling, making ideas like #133 and #679 not possible. And a lot of wasted CPU cycles in general. |
Maybe we can make this a fallback for unsupported slow-types. |
I think the suggested fallback is just to opt-out with the ignore slow types option when publishing. I don't think the issue is compiling TS to JS but rather the extra features of JSR that can only handle opinionated TypeScript. |
|
Oh I see, I've no idea about that, sorry. Hopefully the Deno team will address this issue soon and provide further insight on their plans to improve TypeScript support 🙏 |
JSR sets low score for packages which use type inference:
I've developed quite a few typescript libraries. Never felt a slowdown from inferred types. Moreover, editors like vscode have settings that allow automagically appending inferred types in UI.
Why do we need to rewrite everything now to get proper scores? Because JSR can't be bothered to become fast enough to deduce these types? Feels like a problem that JSR should solve internally, instead of forcing an opinion on users.
The text was updated successfully, but these errors were encountered: