tatethurston / nextjs-routes

Type safe routing for Next.js
MIT License
571 stars 23 forks source link

ability to define what query params can exist on each path #39

Open sachinraja opened 2 years ago

sachinraja commented 2 years ago

This is more of something that would take nextjs-routes to the next level for us, but it's already a great solution. It would be nice if we could augment the query types for each route to add our own. For example, defining a query param called edit on the /user/[id] route to change the page to edit mode.

This is just an feature request right now, I'll try to think of some ideas on how we could do this later.

tatethurston commented 2 years ago

Awesome, I’m looking forward to seeing what you come up with. I’ll think on this too.

tatethurston commented 2 years ago

Would this be for enforcement of query params when writing a new path? Is there another use case I’m not thinking of? Would we want users to have the ability to prevent unspecified query parameters?

Reading query will have to keep the value as optional (as opposed to required like with the path parameters) because a user can always enter a path without the query.

tatethurston commented 2 years ago

Some approaches I can think of:

  1. Making Link and router methods accept a generic argument to type query (effectively a cast)
  2. Providing a configuration file for nextjs-routes that is read when generating types that enables customization
  3. Having an extension point defined so users extend a type defined by nextjs-routes and provide their query overrides
  4. Have a bespoke export on each page that nextjs-routes reads during route generation.
sachinraja commented 2 years ago

I think the configuration file is the best option. Something like this:

// nextjs-routes.config.js
export default {
  routeOverrides: {
    '/user/[id]': {
      queryParams: ['edit', 'add']
    }
  }
}
ChristoRibeiro commented 2 years ago

Just an idea: Call the config file next.routes.js so it will be just after the file next.config.js. Easy to catch on all the files.

tatethurston commented 2 years ago

Another config file option would be extending next.config.js with a “nextjs-routes” key to avoid an extra file. I’ve seen a few community next packages follow this pattern.

Or instead of the nextjs-routes key in the same config object that next uses, we follow the same pattern as @next/mdx

ChristoRibeiro commented 2 years ago

+1 for the suggestion of @tatethurston ! less (file) is more 👏

tatethurston commented 2 years ago

pathpida goes with the bespoke named export approach: https://github.com/aspida/pathpida#define-query---nextjs

BigDog1400 commented 2 years ago

pathpida goes with the bespoke named export approach: https://github.com/aspida/pathpida#define-query---nextjs

I really like this aproach. I am currently using this package, and i have a search page where exist a lot of query params, and it is really difficult right now to use the library without those query params being typed by the package itself.

tatethurston commented 1 year ago

I'm deciding between two approaches:

  1. Named Export
    
    // pages/user/[id].tsx

export interface SearchParams { edit: string; add: string; }


2. Configuration in `next.config.js`:

const withRoutes = require("nextjs-routes/config")({ routes: { '/user/[id]': { searchParams: ['edit', 'add'] } } });

module.exports = withRoutes(nextConfig);



1. Most flexible. This gives users full control over the types, eg they can define optional vs required keys, and string or string[] types. No new syntax, and colocated with the file.

2. Less flexible, unless we introduce additional configuration options. Centralizes all search parameter overrides.

In both cases, it's not quite clear what the DX should be. Required keys are unsafe for reads (eg a user can manually craft a URL), but enforceable for writes. The same problem exists for string / string[]. nextjs-routes can either enforce type safety with distinct types for reads/writes (`string | string[] | undefined` for reads, user defined type for writes), or take user input as is yielding type unsafe behavior. Eg:

typing `edit` or `add` for `/user/[id]` as `string` is always type unsafe for reads, because a user could go to `/users/1?edit=foo&edit=bar` yielding a `string[]` or `/users/1` yielding undefined.

@sachinraja is this still a feature you're still interested in?
ChristoRibeiro commented 1 year ago

Interested about this feature ☝️ The approche 1 is my favorite. Easy to manage types with TypeScript :)

tatethurston commented 1 year ago

@ChristoRibeiro could you tell me more about your use case?

My main concern is how to handle the following, and I'm curious if it applies to your use case:

// pages/user/[id].tsx

export interface SearchParams {
  edit: string;
}

when reading router.query.add, should it be typed as string or string | string[] | undefined? Because a user could enter /users/1?edit=foo&edit=bar yielding a string[] or /users/1 yielding undefined.

For writes (setting the url) the type would be enforced as string (eg for router.push, and Link)

sachinraja commented 1 year ago

Hey @tatethurston I would prefer the next.config.js config.

typing edit or add for /user/[id] as string is always type unsafe for reads, because a user could go to /users/1?edit=foo&edit=bar yielding a string[] or /users/1 yielding undefined.

Is there any way nextjs-routes could first do some runtime validation? TanStack Router does this using zod but we wouldn't need something that complicated for this.

ChristoRibeiro commented 1 year ago

My use case is simple, I have some pages where my users can play with data filters. So my URLs can get these shapes:

Different pages, same filters params. If I can define in one place my data filters params and use them on different pages it could be nice 😊

About types each filter have specifics and default value. Like so:

type Sort = 'asc' | 'desc'
type GroupBy = 'name' | 'age'

type DataFiltersParams = {
  sort: Sort
  groupBy?: GroupBy
}

const dataFiltersParams: DataFiltersParams = {
  sort: 'asc'
}
tatethurston commented 1 year ago

Hey @tatethurston I would prefer the next.config.js config.

typing edit or add for /user/[id] as string is always type unsafe for reads, because a user could go to /users/1?edit=foo&edit=bar yielding a string[] or /users/1 yielding undefined.

Is there any way nextjs-routes could first do some runtime validation? TanStack Router does this using zod but we wouldn't need something that complicated for this.

Certainly not opposed to introducing a runtime solution, but it’s not clear to me from TanStack router’s documentation what advantage their search parameters runtime offers over URLSearchParams for the majority of applications: ultimately the best API you can provide for reads is

  1. string | undefined
  2. string[]
  3. string, with a fallback value.

URLSearchParams get achieves 1 and getAll achieves 2.

Maybe you can point out what I’m missing?

The arbitrary serialization/deserialization formats is neat, but reaching for eg binary serialization of query params seems quite niche, and once you’re customizing at that level, I would expect you to simply write an application specific wrapper around url reads and writes.

Introducing a runtime mirroring Next13s useSearchParams (effectively backporting for page directory usage) seems reasonable.

tatethurston commented 1 year ago

My understanding so far is there are two pain points that folks would like a solution to:

  1. requiring some page writes (link, push, replace) to require specific search params to be set
  2. Search parameter reads that do not need to handle string | string[] — just one or the other.

Is that a correct summary? Does that miss anyone’s ask? Do folks care more about one more than the other?

My inclination is to defer string literal unions and anything else requiring parsing to users, who can apply something like zod or their validation library of choice. Open to push back there though.

janwirth commented 1 year ago

I would also like this feature. My use case is sharing a URL with a baked-in query parameter. That is domain.com/route/[id]?share-key=my-share-key. I want to make sure that the generated link and the URL are in sync.

tran-simon commented 11 months ago

@ChristoRibeiro could you tell me more about your use case?

My main concern is how to handle the following, and I'm curious if it applies to your use case:

// pages/user/[id].tsx

export interface SearchParams {
  edit: string;
}

when reading router.query.add, should it be typed as string or string | string[] | undefined? Because a user could enter /users/1?edit=foo&edit=bar yielding a string[] or /users/1 yielding undefined.

For writes (setting the url) the type would be enforced as string (eg for router.push, and Link)

I think string | string[] | undefined is more accurate and is good enough for most uses. The main goal is to know what params are supported by the route, the primary use case is to not forget some fields when updating the params