Use the type system to verify version again

[lishaduck]

Depends on: #63.

Reverts: 78ed4ac

[lishaduck]

PRing this, but it should be noted that I can't reproduce the as const requirement. It works fine on my machine :)

[lishaduck]

Yeah, I fully reverted, and my example works fine with the old one. I don't think you need to as const at all. TS seems to be able to infer it.

I'll go take a look at the code from Lume.

[lishaduck]

Hmmm. There's a CI failure, but I didn't touch the actual code, just the types. I'll look at it later.

[lishaduck]

Huh. I can reproduce locally, but the diff clearly shows that I didn't change anything.
Well, I got to fly, but I'll look into it later.

[lishaduck]

Figured it out 😀
My example was getting tested, and the types aren't quite right. You have to have another property, it seems.

[lowlighter]

Thanks a lot for looking into this !

[lishaduck]

Hey, so I just finished some more investigation and this turns out to be more of a docs issue :)
It did seem a little weird to use generics here.

Basically, typescript assigns types sequentially and assumes everything is mutable... so it doesn't know how it'll be used ahead of time. Therefore, it assigns the type string b/c it doesn't know the object is an xml_document until later, when it's used. All of my code called it directly, so TS had the context, but the Lume code assigns it to a variable, setting the type in stone. I'm looking into ways to make TS recognize it otherwise, b/c I think I've fixed this before somehow. Maybe just by inlining it? I figure that's the best option, but I'm looking at some casting options as well.

[lishaduck]

Ok, a few options:

a) Don't merge this, lose typesafety (I'm usually a purist, but this clearly trips some people up, so maybe?)
b) Recommend as consting the version or the whole object. TS surprisingly seems fine with as consting dynamic values. (You seem against this, reasonably)
c) Recommend the satisfies operator (i.e., {...} satisfies Partial<xml_document>). (Provides a better error than b.)
d) Recommend as const satisfies Partial<xml_document> (My favorite option, almost certainly too verbose)
e) Recommend casting to the value (Loses typesafety, verbose)
f) Recommend explicitly defining the type : Partial<xml_document> (verbose, not as useful as just using satisfies)
g) Recommend inlining the value into the function when possible.

I'd probably just go with c.

[lishaduck]

Oh, whatever you choose, you won't need the generics, just maybe a partial_xml_document type. Honestly, it doesn't save a keystroke and only saves a character, so I wouldn't do anything there. Just revert 78ed4ac and call it a day (with some docs changes).

[lishaduck]

One thought I just had—does libs follow SemVer? What's the public API? Presumably this is a major? It feels rather mundane, but I guess it is technically breaking. Yeah, I'm very much a purist. Oh well :)
Feel free to make this a patch or a minor. I don't really care, I don't use libs personally. Though do I plan on it soon-ish, probably. I like the APIs 😁

Edit: I now see that libs does follow SemVer.

[lishaduck]

Ok, should be good to go!

[lowlighter]

a) Don't merge this, lose typesafety (I'm usually a purist, but this clearly trips some people up, so maybe?)
b) Recommend as consting the version or the whole object. TS surprisingly seems fine with as consting dynamic values. (You seem against this, reasonably)
c) Recommend the satisfies operator (i.e., {...} satisfies Partial<xml_document>). (Provides a better error than b.)
d) Recommend as const satisfies Partial<xml_document> (My favorite option, almost certainly too verbose)
e) Recommend casting to the value (Loses typesafety, verbose)
f) Recommend explicitly defining the type : Partial<xml_document> (verbose, not as useful as just using satisfies)
g) Recommend inlining the value into the function when possible.

I think it'd be best to go with a mixed solution of a/c:

• Strict typing for parse() (i.e. keeping the revert you've applied so @version: 1.${number} and @standalone: "yes" | "no" which means that end-users will have narrowed values but can choose to use them like they want
• Lax typing for stringify() using document: Partial<Omit<xml_document, "@version"|"@standalone"> & { "@version": string, "@standalone": string}>, and recommend satisfies for better typing from a user-side like you said

The reasoning behind is, in addition to avoiding releasing a new major, is that while I want to enforce strict typing in the internal code of @libs, I don't want to enforce a particuliar way of coding on end-users

I'm already quite annoyed by explicity typing slow-types since I always prefer to let TS infer stuff so I can be less verbose, so I can imagine end-users also being annoyed to find-out that stringify() wouldn't work out-of-the-box if you don't explicitly set typings, especially for a such a detail of standalone/version. I think this is a nice compromise between conveniency and strict typing

[lishaduck]

I think it'd be best to go with a mixed solution of a/c:

• Strict typing for parse() (i.e. keeping the revert you've applied so @version: 1.${number} and @standalone: "yes" | "no" which means that end-users will have narrowed values but can choose to use them like they want
• Lax typing for stringify() using document: Partial<Omit<xml_document, "@version"|"@standalone"> & { "@version": string, "@standalone": string}>, and recommend satisfies for better typing from a user-side like you said

The reasoning behind is, in addition to avoiding releasing a new major, is that while I want to enforce strict typing in the internal code of @libs, I don't want to enforce a particular way of coding on end-users

Ok, I'll do that. I just finish rebasing this a bunch, so I should be ready to edit it again. The stack should now be #66 (so that I don't ruin formatting), #63 (so that the prose wrapping is included), #62 (this one).

I'm already quite annoyed by explicitly typing slow-types since I always prefer to let TS infer stuff so I can be less verbose, so I can imagine end-users also being annoyed to find-out that stringify() wouldn't work out-of-the-box if you don't explicitly set typings, especially for a such a detail of standalone/version. I think this is a nice compromise between conveniency and strict typing

Yeah. I get slow-types (TS 5.5 is releasing with it built in soon), but it's weird. I don't plan on turning it on, none of my projects are slow enough for it to be beneficial.