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

Doesn't take into account rewrites/redirects #15

Open
osdiab opened this issue Nov 11, 2021 · 4 comments
Open

Doesn't take into account rewrites/redirects #15

osdiab opened this issue Nov 11, 2021 · 4 comments

Comments

@osdiab
Copy link
Collaborator

osdiab commented Nov 11, 2021

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

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

osdiab commented Nov 12, 2021

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

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

osdiab commented Nov 17, 2021

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.

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