ckastbjerg / next-type-safe-routes

Never should your users experience broken links again!
MIT License
70 stars 5 forks source link

Doesn't take into account rewrites/redirects #15

Open osdiab opened 3 years ago

osdiab commented 3 years ago

Idk if there's a way to support it easily, but the generated routes don't seem to read the next.config.js to gather valid rewrites and redirects. Maybe can be an extra parameter to the plugin to just specify some extra manual routes in next.config.js, which would be OK - can just pass in the missing values, and its possible to do so without redundant code (define the rewrites as a function, pass it to the nextJS config, and then invoke it to pass the rewrites to this plugin).

So this would be a breaking change - the /plugin import would instead return a function that produces a NextJS plugin, with an objects blob:

// next.config.js
const withPlugins = require("next-compose-plugins");
const nextTypeSafePages = require("next-type-safe-routes/plugin");

function makeRewrites() {
  return [{ source: "/foo", destination: "/bar" }]
}

const config = {
  async rewrites() {
    return makeRewrites(); 
  }
}

const options = {
  additionalPages: makeRewrites().map(r => r.source).filter(s => !s.startsWith("/api")),
  additionalApiRoutes: makeRewrites().map(r => r.source).filter(s => s.startsWith("/api")),
}
module.exports = withPlugins([nextTypeSafePages(options)], config);
ckastbjerg commented 3 years ago

Interesting challenge. I haven't myself made use of of rewrites and redirects in Next.js yet, so haven't had to deal with it.

While your solution seems feasible, might another option be to just parse the next.config.js file? I'm guessing that it would be reasonable to assume that the pattern [{ source: "/foo", destination: "/bar" }] is unique to rewrites.

Might be a bit magical though...

osdiab commented 3 years ago

yeah i think since they allow for any function that returns that array of objects, it's tricky, so thats why I was thinking a simple config option to just state extra existant routes is probably the most applicable/letting the user decide how that's done.

ckastbjerg commented 3 years ago

How urgent do you think this is? Based on the docs, it seems to me that rewrites would be fairly complex to support solidly. There are many edge cases it feels like...

osdiab commented 3 years ago

It's not super urgent for me (there's always a workaround to just cast to TypeSafePage if you're feeling confident), but I agree that trying to figure it out from the redirect/rewrite functions could be tricky, hence my suggestion it just be a manual configuration. But yeah doesn't need to be prioritized highly unless it becomes more pressing for me or someone else.