Skip to content
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

A $safeParseParams which doesn't raise an error immediately #58

Closed
kelvindecosta opened this issue Sep 9, 2024 · 7 comments
Closed

Comments

@kelvindecosta
Copy link

I find myself using $parseParams when I need to check if the current pathname matches a certain route handler (for rendering components in a common layout).

I currently use a try and catch wrapper for this, but I thought it would be a nice addition to this useful library.

@kelvindecosta
Copy link
Author

Some code for reference:

export type InferParams<RP> =
  RP extends RoutesProps<infer _R, infer _C, infer Params>
    ? {
        params: Params["path"];
        searchParams: Params["query"];
      }
    : never;

export function $safeParseParams<
  R extends RouteNodeMap,
  C extends AnyRenderContext,
  P extends ParamRecordMap = ParamRecordMap,
>(routes: RoutesProps<R, C, P>, pathname: string) {
  try {
    return routes.$parseParams(pathname);
  } catch {
    return null;
  }
}

@kruschid
Copy link
Owner

kruschid commented Sep 9, 2024

Thank you for the example. I already had this on my mind. It could be done by adding a meta prop like $parseParams(pathname, {strict: false}) or as you said, a dedicated method $safeParseParams. Whatever is easier to implement and consistent with the rest of the code. I have to look into it.

@kruschid
Copy link
Owner

kruschid commented Sep 9, 2024

I just came up with an idea. However, I can't compare it with your solution because I don't understand the purpose of InferParams. It seems like you didn't intend to paste it here, did you?

The following snippet defines a generic safeCall wrapper function. The arguments it takes are not limited to string-based location paths, but are flexible enough to handle other parameter types. As it's a generic function, it is also compatible with other methods like $parseQuery. This means we do not have to re-implement it for each parsing method separately.

function safeCall<T extends (...args: any[]) => any>(
  fn: T,
  ...params: Parameters<T>
):
  | { success: true; result: ReturnType<T> }
  | { success: false; error: Error } {
  try {
    const result = fn(...params);
    return {
      success: true,
      result,
    };
  } catch (error) {
    return {
      success: false,
      error,
    };
  }
}

safeCall(routes.$parseParams, pathname);

This type of wrapper is probably already included in some utility library. I'll need to check. If not, I may have to bundle it with typesafe-routes.

edit: fn(params) fn(...params)

@kelvindecosta
Copy link
Author

I think the implementation for safeCall makes a lot of sense!

However, I can't compare it with your solution because I don't understand the purpose of InferParams. It seems like you didn't intend to paste it here, did you?

Ah yeah, my bad. I needed it to type the Page components in Next.js.
I can easily do type ThingPageProps = InferParams<routes.thing> and it works like magic.

@kruschid
Copy link
Owner

kruschid commented Sep 11, 2024

I think the implementation for safeCall makes a lot of sense!

I found lodash attempt and several other similar utility functions. However they all have very loose Typescript types. So, I decided to include safeCall as an optional utility function in the next beta release. $safeParseParams and $safeParseQuery methods would probably have a slightly better DX, but I don't see why they should be part of the core features, which are creating and parsing paths.

Ah yeah, my bad. I needed it to type the Page components in Next.js.
I can easily do type ThingPageProps = InferParams<routes.thing> and it works like magic.

Oh, I see. That's nice. We had a similar utility type in the previous versions of typesafe-routes. Would you mind if I incorporate your code into the library? I would rather not consider the type RoutesProps, which your InferParams is based on, to be a part of the public API. That type might be subject to changes, which could break your app after an update. That's why RouteProps is not mentioned in the documentation.

The property names params and searchParams have to be named like that in Next.js, right?

@kelvindecosta
Copy link
Author

kelvindecosta commented Sep 12, 2024

Would you mind if I incorporate your code into the library?

I would not mind at all, please feel free to add it!

The property names params and searchParams have to be named like that in Next.js, right?

Yes, I am using this within a Next.js project.
You can perhaps keep it true to the original structure like so:

export type InferParams<RP> =
  RP extends RoutesProps<infer _R, infer _C, infer Params>
    ? Params
    : never;

I can easily do type ThingPageProps = InferParams<routes.thing>

I was slightly wrong here; it should be InferParams<typeof routes.thing>

@kruschid
Copy link
Owner

kruschid commented Oct 3, 2024

I just ran a few tests, and with TypeScript < 5.0.0, InferParams causes the following error: TS2590 - Expression produces a union type that is too complex to represent. TypeScript 5 is still relatively new (released in May last year), so I think I'll keep the required peer dependency "typescript": ">=4.5.0" in package.json but add a note in the documentation.

For versions 4.5-4.9, the parameters can be inferred as follows:

type InferredParams = {
  query: ReturnType<typeof r.parent.child.grandchild.$parseQuery>;
  path: ReturnType<typeof r.parent.child.grandchild.$parseParams>;
}

This isn't as convenient as the utility function, but at least it's compatible with older versions.

This is just for documentation purposes, in case someone looks for the information in the issues. The workaround will be included in the documentation.

I'm closing this issue and will publish a new version based on the results of this discussion. Thanks again. ✌️

@kruschid kruschid closed this as completed Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants