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

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

Open
sachinraja opened this issue Jul 12, 2022 · 18 comments
Open

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

sachinraja opened this issue Jul 12, 2022 · 18 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@sachinraja
Copy link
Contributor

sachinraja commented Jul 12, 2022

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
Copy link
Owner

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

@tatethurston
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Owner

tatethurston commented Jul 18, 2022

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

@tatethurston tatethurston mentioned this issue Jul 18, 2022
Closed
@ChristoRibeiro
Copy link

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

@tatethurston
Copy link
Owner

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

@BigDog1400
Copy link

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
Copy link
Owner

tatethurston commented Nov 21, 2022

I'm deciding between two approaches:

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

export interface SearchParams {
  edit: string;
  add: string;
}
  1. 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
Copy link

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

@tatethurston
Copy link
Owner

tatethurston commented Nov 22, 2022

@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
Copy link
Contributor Author

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
Copy link

ChristoRibeiro commented Nov 24, 2022

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

  • /product-data?sort=asc&groupBy=name
  • /products-data?sort=desc&groupBy=age

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
Copy link
Owner

tatethurston commented Nov 24, 2022

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
Copy link
Owner

tatethurston commented Nov 24, 2022

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
Copy link

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
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants