fix: update types

[gameroman]

🔗 Linked issue

📚 Description

Add description?: never to DocumentedModuleReplacement for easier use with typescript

This is to avoid doing things like this:

https://github.com/npmx-dev/npmx.dev/blob/d80c709c30b41ac68bb91b6524fabe59477417d2/app/components/Package/Replacement.vue#L17-L19

[43081j]

I'm not sure this is right. Documented replacements can have their own URL. They just don't right now.

But it was typed this way because some may need their own one day that isn't the same as the mapping URL.

Don't have an example off the top of my head though

[gameroman]

This is description, not URL

[43081j]

Sorry it also applies to the description 😅

It is possible we want a description on any given replacement. We just happen to not have any right now on documented ones.

If we remove this, we're locking ourselves out of the option in future without a breaking change

[gameroman]

In that case we can add description?: string to ModuleReplacementLike instead

[43081j]

Actually now that I've looked properly, I'm a bit lost 😅

Documented replacements already don't have descriptions. So the types are already right, no?

[gameroman]

Yes, they don't, this just allows you to do this:

https://www.typescriptlang.org/play/?#code/JYOwLgpgTgZghgYwgAgILIN4Chm+cAEwC5kQBXAWwCNoBuHPEOCiAfhIGcwpQBzegL5YsoSLEQoAQpga5CJctTqzSzNgogA3ZUOFYYZEAjDAA9iGQwAFGQ7QS6AD7JJASk7c+MvMigQwZFAWttAAdEwsyKysyADksVhCQA

And not have typescript complain

https://www.typescriptlang.org/play/?#code/JYOwLgpgTgZghgYwgAgILIN4Chm+cAEwC5kQBXAWwCNoBuHPEOCiAfhIGcwpQBzegL5YsoSLEQoAQpga5CJctTpYhwrDDIgEYYAHsQyGAAoyHaCXQAfZJICUnbnxl5kUCGDJQDp6ADomLMisrMgA5KEqWEA

And avoid having to do something like this

https://www.typescriptlang.org/play/?#code/JYOwLgpgTgZghgYwgAgILIN4Chm+cAEwC5kByOUgbhzxDgFsIB+EgZzClAHNqBfLLKEixEKAEKYauQiVIAjKln4CsMAK4gEYYAHsQyGAAo1raCXQAfZGICUbDt0l5kUCGDVR9J6ADpCyAF4gsgVkJjJSZBJvKB86RjDw0lIlLCA

Or this

https://www.typescriptlang.org/play/?#code/JYOwLgpgTgZghgYwgAgILIN4Chm+cAEwC5kByOUgbhzxDgFsIB+EgZzClAHNqBfLLKEixEKAEKYauQiVIAjKln4CsMAK4gEYYAHsQyGAAo1raCXQAfZGICUbDt0l5kUCGDVR9pOo1L59JtDITMiBUAB0PihMIaR+sqRKWEA

[43081j]

this was by design though since all consumers should be narrowing the tagged union really.

having description: never doesn't feel right since the type shouldn't have the property at all.

[gameroman]

description?: never is just saying that description is always undefined but allows you to use description property in union contexts

[gameroman]

And the difference between description?: never and description: undefinedis thatdescription?: nevershows thatdescription` does not actually exists as a property on an object

[43081j]

i understand what it does - but it declares that the type has a property description which isn't true.

the value is never.

i know this seems picky but i really believe this is just a workaround to avoid having to narrow the types properly. the current types are correct imo

[gameroman]

@bluwy @ghostdevv what do you think?

[joaopedrodcf]

having a type never seems quite strange to me you did ?? "" because it can be undefined no ?

[gameroman]

Yes, description can be undefined on ModuleReplacement, but because DocumentedModuleReplacement does not have the description property at all typescript will complain that

Property 'description' does not exist on type 'DocumentedModuleReplacement'

By adding description?: never we are allowing us to access description on ModuleReplacement

Also I want to note that description?: never and description?: undefined are exactly the same thing

[43081j]

we're not comparing never and undefined though.

// this
interface X {
  prop?: never;
}

// is not the same as this
interface X {
  // no prop at all
}

the types are correct right now and are doing their job by making you narrow the union correctly.

it may feel like a pain but the documented replacements don't have a description property at all. setting them to never is just a workaround to avoid having to narrow the type.

[gameroman]

But why would you want to narrow the type for the description that is already optional on other types?

[43081j]

Because documented replacements don't have a description. It isn't optional, they just don't have one at all.

So you correctly have to narrow the type to ones which do have a description.

This is just a workaround to avoid having to do the narrowing

[gameroman]

I would say this is actually a very common way to do this when dealing with type unions, for example

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/083439f50a07de259eb9690cb0a3d8121f7c60bc/types/npmcli__arborist/index.d.ts#L441-L461

playground

[gameroman]

What would be yours proposed solution?

[gameroman]

I’m JavaScript it doesn’t matter if property is undefined or it doesn’t exist