Skip to content

jsr publish --fix #15

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

Open
lucacasonato opened this issue Feb 28, 2024 · 7 comments
Open

jsr publish --fix #15

lucacasonato opened this issue Feb 28, 2024 · 7 comments
Labels
cli publishing Problems with publishing

Comments

@lucacasonato
Copy link
Member

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.

@KnorpelSenf
Copy link

Why would --fix not be the default?

@lucacasonato lucacasonato added this to JSR Mar 4, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in JSR Mar 4, 2024
@lucacasonato lucacasonato moved this from Needs Triage to Ready in JSR Mar 4, 2024
@paulmillr
Copy link

paulmillr commented Apr 26, 2024

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

@vwkd
Copy link

vwkd commented May 5, 2024

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.

@KnorpelSenf
Copy link

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

@Swimburger
Copy link

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.

@typed-sigterm
Copy link

--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.

@omar-azmi
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli publishing Problems with publishing
Projects
Status: Ready
Development

No branches or pull requests

8 participants