fix: add missing redirects#2333
Conversation
Fixes expressjs#2327 Fixes expressjs#2328 Fixes expressjs#2332
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jonchurch
left a comment
jonchurch
left a comment
There was a problem hiding this comment.
Lets hold off for a moment on adding code to do the html stripping.
I have put in place cloudflare redirect rules to do the .html stripping to at least fix the immediate issues.
So I want to enumerate the actual issues we are seeing rn, as I see the /blog/posts.html to /en/blog redir and am unaware of the specifics there. The cloudflare stripping I put into place wont do that particular one, as an example. nor the api.html => 5x/api redir.
If we can get an accounting of what redirs we need, lets figure out the best path forward from there.
|
|
||
| const redirects = { ...blog, ...api_v2, ...pages }; | ||
| const api = localizedPages({ | ||
| 'api.html': '5x/api/', |
There was a problem hiding this comment.
Looks like we are missing this /api.html => [latest]/api redir, good catch
The other Nx/api.html are just the html stripping.
| }; | ||
|
|
||
| const pages = { | ||
| '/changelog/4x.html': 'https://github.com/expressjs/express/releases', |
There was a problem hiding this comment.
we do need this one too. My cloudflare rule has an ignore setup for /en/changelog/4x.html but not for the the form without /en/. I can update it to also let this one pass through.
|
I have added redirects for all (at least I hope so) The CF redirects don't handle most resources that were accessible without language prefix. These include:
|
|
Okay, I made the creation of redirects without language dynamic so they automatically redirect to English. We don’t really need /api/ to point to /en/5x/api/; /en/api/ is enough since it points to the latest version. The /blog/posts.html to /en/blog one was an oversight—thanks for adding it. I think there shouldn’t be any more broken routes @expressjs/docs-captains I’m going to merge it so there are no more broken URLs from this migration that weren’t accounted for. If there are still any broken URLs, please report them |
|
@krzysdz thanks! |
| "scripts": { | ||
| "dev": "astro dev", | ||
| "build": "astro build", | ||
| "build": "NODE_OPTIONS=--max-old-space-size=4096 astro build", |
There was a problem hiding this comment.
This doesn't work on Windows.
3 participants
Cherry picked 879c8d8 from #2324
Fixes #2327
Fixes #2328
Fixes #2332
Fixes #2343