fix: PathParams type

[jorroll]

This pull request fixes the PathParams type.

The example shown in the docs has a type error in Typescript 4.4/4.5.

type Params = PathParams<"/greeting/:firstName/:lastName">;
// => { firstName: string; } & { firstName?: string | undefined; }

The expected type should include lastName: string.

Here's a typescript playground reproduction.

This merge request fixes the issue so that PathParams is properly typed.

As a side note, it's unclear why PathParams is defined as

export declare type PathParams<S extends string> = {
    [P in SplitRoute<S>[number]]: string;
} & {
    [P in SplitRoute<S>[number]]?: string;
};

instead of simply

export declare type PathParams<S extends string> = {
    [P in SplitRoute<S>[number]]: string;
};

Since {someKey: string} & {someKey?: string} is always equivalent to just {someKey: string}.

[jorroll]

This was my first time submitting a PR using the new VSCode editor built into Github. Worked well.

[gzuidhof]

Thank you! I think it's almost there, the only part that is not working now are optional path params, although I'm starting to doubt they ever worked correctly. I think they're not used very frequently. I imagine that is what the [P in SplitRoute<S>[number]]?: string; was for.

[gzuidhof]

I think CI is failing due to an unrelated reason: we were pinning to a specific version of a library that hadn't been released yet, I can fix that.

[gzuidhof]

Forgot to add this accompanying screenshot, the param with the question mark should be optional. Happy to merge this PR anyway of course as it's already an improvement :)

[jorroll]

I think it's almost there, the only part that is not working now are optional path params, although I'm starting to doubt they ever worked correctly.

Interesting. I wasn't aware of the optional params feature and I didn't realize that so much documentation was behind the link to the path-to-regexp library.

Well modifying this PR to support optional path params should be easy enough. I haven't tested, but I think this code will handle optional params correctly:

/** Split type `"one/:two/:three?/:four"` into `["one", ":two", ":three?", ":four"]` */
type SplitPath<S extends string> =
    string extends S ? string[] :
    S extends `${infer A}/${infer B}` ? [A, ...SplitPath<B>] :
    [S];

/** Convert type `"one" | ":two" | ":three?" | ":four"` into `"two" | "four"` */
type ExtractRequiredParams<S extends string> =
    string extends S ? string :
    S extends `:${infer A}?` ? never :
    S extends `:${infer A}` ? A :
    never;

/** Convert type `"one" | ":two" | ":three?" | ":four"` into `"three"` */
type ExtractOptionalParams<S extends string> =
    string extends S ? string :
    S extends `:${infer A}?` ? A :
    never;

/** 
 * Convert a route path string literal type such as "one/:two/:three?/:four" 
 * into a params interface like `{ two: string; three?: string; four: string }`
 */
export type PathParams<S extends string> = {
    [P in ExtractRequiredParams<SplitPath<S>[number]>]: string;
} & {
    [P in ExtractOptionalParams<SplitPath<S>[number]>]?: string;
};

I'm happy to update this PR with the necessary changes to support optional route params. This being said, having glanced over the path-to-regexp library now, it looks like there are a lot of features in there that aren't currently handled by the PathParams type. It's not clear to me if typescript string literal types are capable of accurately modeling the path-to-regexp options, but considering someone built a SQL database using string type literals, I'm guessing it's possible. I'm not personally interested in trying to flesh out this PR to fully model all the path-to-regexp options though. Are you still interested in merging it?

[gzuidhof]

I really appreciate the work so far, I think when we support optional parameters and other simple regexes we will cover 99% of cases which is good enough. The browser and CF workers are shipping their own native path-to-regexp equivalent soon (I forget the name) which I think is a bit more limited.

When someone wants to use the advanced features (e.g. using actual patterns) they can annotate the type of that themselves (at worst something like Record<string,string>.

I'm currently not near a computer so I'm not able to give the latest revision a test run but will do so after Christmas :)

Thank you again!

On Fri, 24 Dec 2021, 09:39 John, @.***> wrote:

I think it's almost there, the only part that is not working now are optional path params, although I'm starting to doubt they ever worked correctly.

Interesting. I wasn't aware of the optional params feature and I didn't realize that so much documentation was behind the path-to-regexp library link.

Well modifying this PR to support optional path params should be easy enough. I haven't tested, but I think this code will handle optional params correctly:

/** Split type "one/:two/:three?/:four" into ["one", ":two", ":three?", ":four"] /type SplitPath<S extends string> = string extends S ? string[] : S extends ${infer A}/${infer B} ? [A, ...SplitPath<B>] : [S]; /* Convert type "one" | ":two" | ":three?" | ":four" into "two" | "four" /type ExtractRequiredParams<S extends string> = string extends S ? string : S extends :${infer A}? ? never : S extends :${infer A} ? A : never;/* Convert type "one" | ":two" | ":three?" | ":four" into "three" /type ExtractOptionalParams<S extends string> = string extends S ? string : S extends :${infer A}? ? A : never;/* * Convert a route path string literal type such as "one/:two/:three" * into a params interface like { two: string; three: string } */export type PathParams<S extends string> = { [P in ExtractRequiredParams<SplitPath<S>[number]>]: string;} & { [P in ExtractOptionalParams<SplitPath<S>[number]>]?: string;};

I'm happy to update this PR with the necessary changes to support optional route params. This being said, having glanced over the path-to-regexp library now, it looks like there are a lot of features in there that aren't currently handled by the PathParams type. It's not clear to me if typescript string literal types are capable of accurately modeling the path-to-regexp options, but considering someone built a SQL database using string type literals https://github.com/codemix/ts-sql, I'm guessing it's possible. I'm not personally interested in trying to flesh out this PR to fully model all the path-to-regexp options though. Are you still interested in merging it?

— Reply to this email directly, view it on GitHub https://github.com/SunderJS/sunder/pull/4#issuecomment-1000725981, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH5ZFQBKOTIHRQVHMSAMCLUSQWURANCNFSM5KRTXDOA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

[jorroll]

(FYI, haven't forgotten about this--just been busy with the holidays. Might be another week or two before I can follow up.)

[jorroll]

Ok, I've updated this PR to support optional params in addition to required params. I think it's ready to merge.

Separately, I think it's possible that the Router methods should have their types updated to better support overriding the params (at the moment you might need to cast to unknown first like params as unknown as InterfaceYouWant), but I haven't had time to test yet and it seems like a separate issue.

Aside: I was able to submit and update this PR entirely using Github's new vscode web editor. Pretty nice!

[jorroll]

Looks like the test is failing because PathParams doesn't support the + param modifier so I tihnk PathParams<"/post/:path+"> is resulting in the interface { "path+": string }.