wouter icon indicating copy to clipboard operation
wouter copied to clipboard

Typescript Improvements possible (in v2.8.0-alpha.2)

Open HansBrende opened this issue 2 years ago • 2 comments

First of all... bravo for making this library. I became a little frustrated with react-router last weekend and started redesigning everything in it... only to come here and find this library, which I find has very similar ideas to my homebrewed stuff! There are just a couple areas in the typings where my library caught some edge cases and yours does not... so I will share some happy paths and sad paths below (The quoted errors are the errors received from my homebaked implementation. Anything with a red ❌ is where the behavior is unexpectedly different). Hopefully this screenshot gives you some ideas: Screen Shot 2022-01-24 at 7 42 11 PM

HansBrende avatar Jan 25 '22 01:01 HansBrende

To go into more detail: there are 2 nice typescript errors that occurred in mine that are not occurring in yours:

(1) Dynamic paths (where the path parameters cannot be deduced) I typed as

type DynamicPathParams = {
    readonly [key in string]?: string
}

Rather than:

type DynamicPathParams = { // BAD (even though this is the same behavior of Record<string, string>) ❌
    [key in string]: string
}

My full definition, for reference, was actually conditional based on the type of key present:

// Can be used EITHER as:
//   1. Params<string> (keys are always conditionally present) OR 
//   2. Params<ParamParseKey<PathString>> (keys are unconditionally present if PathString is known)
export type Params<Key extends string = string> = string extends Key ? {
    readonly [key in Key]?: string;
} : {
    readonly [key in Key]: string;
};

type ParamParseKey<PathString extends string> =
    PathString extends `${infer L}/${infer R}` ? ParamParseKey<L> | ParamParseKey<R>
        : PathString extends `:${infer Param}` ? Param
            : PathString extends '*' ? '*' : never

That way (as in the happy path version of the same thing in the screenshot) you have to use the param! typescript syntax to coerce the type from a possible undefined if the route parameter names are not inferable.

(2) Multiple children of a route (which I am assuming are not allowed if one is a function) are not showing as an error. This is probably because of this React typings bug: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/29307 Where anything is allowed to be a ReactNode due to its inclusion of {} | ReactNodeArray in the definition. I fixed this same problem in my own homebaked project by defining:

type StrictReactNode = ReactChild | StrictReactNode[] | ReactPortal | boolean | null | undefined;

(exactly the same definition of ReactNode except omitting {} from the union) then doing...

export type RouteChildren<PathString extends string> = 
StrictReactNode | ((params: Params<ParamParseKey<PathString>>) => ReactNode);

HansBrende avatar Jan 25 '22 02:01 HansBrende

👍

cbbfcd avatar Jan 27 '22 10:01 cbbfcd

Hi @HansBrende, sorry for the late reply (it's been 10 months and I feel sad that I hadn't responded right away). Thanks for the suggestions!

It seems like 2) has been fixed already and it does give an error now: image

Regarding 1), I'll see what I can do to fix this.

molefrog avatar Nov 02 '22 17:11 molefrog

@molefrog awesome!

FYI: (2) has been fixed only for React 18 and upwards. So if you do want the correct error on React 17, you could backport their definition from the fix (which is the same definition as mine).

HansBrende avatar Nov 04 '22 16:11 HansBrende