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

Is there a method to apply a solution incrementally to certain paths? #172

Open
saengmotmi opened this issue Oct 11, 2023 · 4 comments
Open

Comments

@saengmotmi
Copy link

saengmotmi commented Oct 11, 2023

Hello. While searching for a consistent path setting tool for our Next.js project, I came across nextjs-routes and pathpida. Thank you for creating such wonderful libraries. If I start a new project, I will definitely install these libraries.

Personally, I prefer the approach of nextjs-routes that provides type constraints without affecting runtime. However, as far as I understand, I couldn't find a way to do that. Probably because it generates types and applies them to the entire project.

I want to introduce this tool to address type-related issues with useRouter, but to directly implement it in our project, I would have to resolve numerous type errors. I need a method to apply it incrementally. Do you have any good suggestions?

Actually, the most painful part for me is that the type of the route.query object returned from useRouter falls to string | string[] | number. In most cases, I don't need an array, and it's either string or number, but having to typecast is very annoying. Even if it's not through a gradual introduction, if you know a way to solve this problem, I'd appreciate your help.

If there's any way I can assist in solving this issue, please let me know. Thank you for reading and once again, thank you for creating such amazing libraries.

@tatethurston
Copy link
Owner

tatethurston commented Oct 11, 2023

Happy to brainstorm some ideas for incremental adoption. What would be your ideal approach here? Adoption by route, file or something else?

Actually, the most painful part for me is that the type of the route.query object returned from useRouter falls to string | string[] | number. In most cases, I don't need an array, and it's either string or number, but having to typecast is very annoying.

This is actually unsafe to type differently, because a user can type any string for a url, with arbitrary values or missing values for the query string, which would cause an application error if your code only expects a subset of those options. You can unsafely cast if you don’t care, but I can’t make it the default because of the safety issue. The correct approach here is likely a runtime solution, that parses the query string into your desired form. Related: #39

@saengmotmi
Copy link
Author

@tatethurston I've read the issue you linked. This is just out of curiosity and an idea, but is it possible to have an option like useRouter<T> where you can provide types for the path and query? In most cases, we would access the path and other details through useRouter. Have you ever considered this approach?

and... Would it be better to close this issue and move to the original one?

@tatethurston
Copy link
Owner

@saengmotmi useRouter currently does accept a type argument -- it accepts the path: https://github.com/tatethurston/nextjs-routes#query

Eg:

const router = useRouter<"/foos/[foo]">();
// query is now typed as `{ foo?: string | undefined }`
router.query;

I don't think providing a query type argument is worthwhile. Eg this:

const router = useRouter<"/foos/[foo]", { foo: string}>();
// query is now typed as `{ foo: string  }`
router.query;

Doesn't meaningfully give you anything over casting, which is already possible:

const router = useRouter<"/foos/[foo]", { foo: string}>();
// query is now typed as `{ foo: string  }`
router.query as { foo: string};

@saengmotmi
Copy link
Author

saengmotmi commented Nov 8, 2023

@tatethurston

I believe there is merit in providing a query string option. This is because query strings usually have different possibilities on a per-page basis, and if we manage (and make exportable) the type interface for query strings within the file that defines the page in Next.js, and if we can use that with useRouter, it would be beneficial.

It's advantageous to include the query string in the query returned by useRouter, especially from the perspective of colocation, and as the use of router.query spreads across multiple places, the need for type assertions increases, leading to fatigue.

eg.

// /pages/[detail].tsx
// QueryString Interface with pages
interface QueryString {
  a: number;
  b: number;
}

// page codes ...
// /components/pages/Detail.tsx - if use query string parameter

const Detail  = () => {
  const router = useRouter<"/[detail]", QueryString>()

  // ...
  const a = router.query.a

  // ...
  const b = router.query.b
}
// /components/pages/Detail.tsx - if dont use query string parameter

const Detail  = () => {
  const router = useRouter<"/[detail]">()

  // ...
  const a = (router.query as QueryString).a

  // ...
  const b = (router.query as QueryString).b
}

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