jsr icon indicating copy to clipboard operation
jsr copied to clipboard

`jsr publish --fix`

Open lucacasonato opened this issue 1 year ago • 7 comments

I think that we can auto fix the majority of slow types diagnostics (missing return types, and explicit class types) by using TSC inference and then fixing up the source code with the explicit types gotten from TSC.

This is a bit of work, but would make supporting types in your package a matter of jsr publish --fix (or deno lint --fix). Right now the wall of slow types errors could be a bit daunting.

lucacasonato avatar Feb 28 '24 14:02 lucacasonato

Why would --fix not be the default?

KnorpelSenf avatar Mar 01 '24 21:03 KnorpelSenf

@KnorpelSenf I don't want jsr to mess with my shit by default

paulmillr avatar Apr 26 '24 22:04 paulmillr

Not necessarily implied by the description, but I too suspect modifying code on-the-fly while publishing might not be a good idea.

This could open a can of worms where the code in the repo and the published code start to diverge, if not all modifications are bijective transformations and the published code is ever used as the repo code. Maybe unlikely to occur, but one could imagine some scenario where the published code is used to restore a repo that was somehow deleted or lost. Maintaining this subtle contract might be difficult as TypeScript and JavaScript evolve over the years, as it takes only one non-bijective transformation to break it.

I'd prefer if it modified the local copy of the code such that it can be committed to the repo and publish can remain a simple upload of the exact local code.

vwkd avatar May 05 '24 11:05 vwkd

So it seems like implementing this in deno lint --fix would be the best option

KnorpelSenf avatar May 06 '24 13:05 KnorpelSenf

So it seems like implementing this in deno lint --fix would be the best option

If the jsr publish command does linting, there should also be a jsr lint or jsr publish lint, including a --fix argument. deno lint --fix also makes sense, but jsr users should not have to install deno.

Swimburger avatar May 22 '24 23:05 Swimburger

--fix may cause confusion with linters. Maybe a more specified name like --prebuild-slow-types is better to understand. And to make the final publish command shorter, we can add publishConfig to jsr.json, like package.json.

typed-sigterm avatar Mar 12 '25 02:03 typed-sigterm

Personally, I really want to see some level of inference for untyped variables, without the use of linter or a code-generator. That's because there are many places where I intentionally leave my variable untyped, so that its value is inferred by the return type of the function or class that is being assigned to it.

Take the following basic example where jsr publish complains about slow types:

./deno.json

{
	"name": "@yabadaba/doo",
	"version": "0.1.0",
	"exports": "./mod.ts"
}

./mod.ts

/** module comment.
 * 
 * @module
*/

/** doc comment of `abcFactory`. */
export const abcFactory = (): string => ("") // <- fully typed

/** doc comment of `xyzClass`. */
export class XyzClass {}                     // <- fully typed

/** doc comment of `defaultAbc`. */
export const defaultAbc = abcFactory()   // <- jsr complains about slow-type, and deno-doc fails to infer, despite being easily inferable

/** doc comment of `defaultAbc`. */
export const defaultXyz = new XyzClass() // <- jsr complains about slow-type, but deno-doc infers correctly

Now if I were to publish this via

deno publish --dry-run --allow-dirty

It will complain that both defaultAbc and defaultXyz are untyped.

So now, I'll have to either:

  1. Surrender the type-safety of my variables by concretely declaring their types:

    export const defaultAbc: string = abcFactory()
    export const defaultXyz: XyzClass = new XyzClass()
    
    • If I were to update the return type of abcFactory to ("a" | "b" | "c") (a subtype of the orginal), the typed variant of defaultAbc will not throw a type-error since string is a super type. And as a result, I as the developer, may not notice and forget to update the definition of defaultAbc, leading to errors and wrong usage by the consumers of the library.
    • The same can happen to the defaultXyz variable: if I introduce a class EfgClass extends XyzClass { }, and then assign new EfgClass() to defaultXyz, but forget to update its type declaration, no type-error will be thown, but it will very likely not be what I as the developer intended.
  2. Use the verbose ReturnType for typing exported variables.

    export const defaultAbc: ReturnType<typeof abcFactory>= abcFactory()
    

    This will become extremely verbose for functions that have generic type-parameters, and even more verbose if those type-parameters are bound to the function's parameters.

If jsr publishing would permit even a tiny-level of type-inference, many of these inconveniences will fade away.

My personal approach for type-inference of untyped exports would have been to collect all untyped exports, run them through the type checker, and for the symbols that could be inferred, store their inferred types in a json-like object (in-memory, so that nothing is generated on the filesystem), then ship it with the package generated for jsr.io, and finally allow jsr.io to read the inffered-types json file and make appropriate additions to the docs that are generated/rendered for the package.

omar-azmi avatar Mar 12 '25 04:03 omar-azmi