-
-
Notifications
You must be signed in to change notification settings - Fork 186
fix: configure general rule for trailing slash #649
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
43081j
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it does work, so seems like a good change to me
to get it to work in dev, we'd need to configure nuxt's router.options.strict to false i think, but that seems to break other things. so for now, maybe we just don't bother in dev and figure it out in a separate PR
|
this is good we can also add a global route middleware, dev only, which redirects |
|
@danielroe |
|
it'll be more performant to do the cdn-level redirect as opposed to middleware |
|
Done |
|
@danielroe can this not be accomplished with nitro (not a blocker!) |
|
absolutely it should be self-hostable - I'm not aware of anything at all in the project that's vercel-specific though. 🤔 all the config for vercel cache is only enabled when deployed on vercel. that's why I extracted it into a module. you should be able to build + run locally with no issues. this shouldn't add another, if done correctly. |
|
It sounds like we should enable middleware not only for dev, but for non-Vercel |
|
i think we have to be careful there are some providers, like netlify(?) that add trailing slashes for static routes and cause infinite loops maybe better to sanitise package name to remove slash on package page? |
|
It's not ideal to have one page with two different paths. Plus, it might be a problem for search engines (npmx have canonical URLs, but redirects are still better) Also, this will be a very targeted fix, but the problem will persist on all other pages and potentially flare up later |
|
we should add a rel=canonical for sure and we already have a util to extract thd package name from path. I think we just change that I was also working on a PR to make Vue Router extract params which is ultimately what we'd want I think, but I'm blocked for now |
|
To clarify: it would be better if we won't redirect, but instead show the correct page for both |
|
yes there is a nuxt config option to respect trailing slashes but it has caused so many issues with some providers that I'd prefer to leave it off if we can |
|
I've created a new PR (#673). Should we close this one or leave it for further discussion and a final decision? |
|
I would merge this one too, as a nice-to-have |
Trailing slash setup. For example I often visit a page with a slash in the end and see a 404 error, even though the package exists. Now it will redirect.
I did this using vercel config. I'm a little concerned that this approach won't work during local development, but it seems better than leaving things as they are. Any better ideas would be appreciated [if needed]
Closes #648