feat: rewriting (experimental) #10867

ematipico · 2024-04-24T12:14:10Z

Changes

This is the culmination of the rerouting implementation as experimental.

Here's the RFC: https://github.com/withastro/roadmap/blob/feat/reroute/proposals/0047-rerouting.md

This PR is meant to be merged for v4.8.0.

Testing

Current CI should pass

Docs

cc @withastro/maintainers-docs for feedback!

I will need your help go understand how to document the feature, because this involves Astro pages and middleware.

changeset-bot · 2024-04-24T12:14:19Z

🦋 Changeset detected

Latest commit: ff5602a

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

bluwy

Generally this seems to look good to me. Is there anything specific you'd also like review on? As I think the other PRs are already reviewed so we don't need a full review here.

packages/astro/src/core/render-context.ts

packages/astro/src/core/middleware/callMiddleware.ts

matthewp · 2024-04-26T18:01:46Z

Bikeshedding time! Based on some research I think these are the names various frameworks / server give this feature:

Tool	Name	Notes
Next.js	rewrite	Config-level
Sveltekit	reroute
Nuxt		No support
Remix		No support
SolidStart		No support
Hono		No support
Nginx	rewrite
Apache	rewrite
Caddy	rewrite

natemoo-re · 2024-04-26T18:42:24Z

Great context to add @matthewp! +1 to calling this feature rewrite.

sarah11918 · 2024-05-01T15:43:41Z

Just leaving a comment in response to @ematipico 's request re: documentation.

UPDATE: Would love to use Ben's #10858 as a model for experimental feature documentation:

Show how to enable the experimental flag
Provide any config/basic usage instruction right here
Link to the RFC for more info and to encourage discussion/get feedback!

While the feature is experimental, and subject to change, it helps keep the spread in docs controlled for where we need to keep constantly updating. It also stops us from writing too much before the feature is fully finalized. (And, every change we make means the translators JUMP on it... so it helps to wait until we're sure!)

BUT (although not my preference): if we REALLY think it's more content than we can reasonably fit in the just experimental domain config, we can add another section to the bottom of the Middleware page as Rerouting (experimental) just like the Internationalization page has domains (experimental) at the end.

And since we do have experimental domains at the end of the internationalization page, we could get away with the i18n-specific usecase of rerouting, linking back to the main entry on rerouting. This wouldn't need to explain anything, just show an example of using the (experimental) rerouting feature to reroute to a different language page content if one page isn't translated.

ematipico · 2024-05-06T09:17:17Z

Commit 128bbcb (#10867) changes everything to use "rewrite"

sarah11918

Thanks @ematipico , the docs part of this is looking great! Just a few suggestions to see what you think!

packages/astro/src/@types/astro.ts

* chore: implement reroute in dev * chore: revert naming change * chore: conditionally create the new request * chore: handle error * remove only * remove only * chore: add tests and remove logs * chore: fix regression * chore: fix regression route matching * chore: remove unwanted test

* feat: rerouting in ssg * linting

* feat: implement reroute in dev (#10818) * chore: implement reroute in dev * chore: revert naming change * chore: conditionally create the new request * chore: handle error * remove only * remove only * chore: add tests and remove logs * chore: fix regression * chore: fix regression route matching * chore: remove unwanted test * feat: reroute in SSG (#10843) * feat: rerouting in ssg * linting * feat: rerouting in ssg * linting * feat: reroute for SSR * fix rebase * fix merge issue

* feat: implement reroute in dev (#10818) * chore: implement reroute in dev * chore: revert naming change * chore: conditionally create the new request * chore: handle error * remove only * remove only * chore: add tests and remove logs * chore: fix regression * chore: fix regression route matching * chore: remove unwanted test * feat: reroute in SSG (#10843) * feat: rerouting in ssg * linting * feat: rerouting in ssg * linting * feat: reroute for SSR * fix rebase * fix merge issue * feat: implement the `next(payload)` feature for rerouting * chore: revert code * chore: fix code * Apply suggestions from code review Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> --------- Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

packages/astro/src/@types/astro.ts

.changeset/pink-ligers-share.md

sarah11918 · 2024-05-07T12:26:59Z

.changeset/pink-ligers-share.md

+})
+```
+
+When enabled, you can use `rewrite()` to **render


Just checking on this formatting here. Did you mean to split up into two lines like this?

.changeset/pink-ligers-share.md

packages/astro/src/@types/astro.ts

packages/astro/src/core/middleware/callMiddleware.ts

bholmesdev

Left some comments on error messaging and code nits. Overall, this is very well-considered for an experimental release! Appreciate defensive checks like the infinite loop counter as well. You're the re-right man for the job.

packages/astro/src/core/app/pipeline.ts

packages/astro/src/core/build/generate.ts

packages/astro/src/core/app/pipeline.ts

packages/astro/src/core/render-context.ts

packages/astro/src/vite-plugin-astro-server/pipeline.ts

packages/astro/test/fixtures/reroute/src/pages/reroute.astro

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> Co-authored-by: Matthew Phillips <matthew@skypack.dev> Co-authored-by: Ben Holmes <hey@bholmes.dev>

.changeset/pink-ligers-share.md

sarah11918

Left one tiny suggestion for the first line of the changeset, otherwise now approving for docs! 🥳

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

Merge day

ematipico added this to the 4.8.0 milestone Apr 24, 2024

github-actions bot added pkg: astro Related to the core `astro` package (scope) docs pr A PR that includes documentation for review semver: minor Change triggers a `minor` release labels Apr 24, 2024

github-actions bot previously requested changes Apr 24, 2024

View reviewed changes

ematipico force-pushed the feat/reroute branch from 465f168 to e2d9f3a Compare April 24, 2024 12:14

bluwy approved these changes Apr 25, 2024

View reviewed changes

packages/astro/src/core/render-context.ts Outdated Show resolved Hide resolved

ematipico commented Apr 25, 2024

View reviewed changes

packages/astro/src/core/middleware/callMiddleware.ts Outdated Show resolved Hide resolved

ematipico force-pushed the feat/reroute branch from 128bbcb to f1d5404 Compare May 6, 2024 09:17

ematipico changed the title ~~feat: rerouting (experimental)~~ feat: rewriting (experimental) May 6, 2024

sarah11918 reviewed May 6, 2024

View reviewed changes

ematipico requested a review from sarah11918 May 7, 2024 12:20

ematipico and others added 10 commits May 7, 2024 13:20

feat: reroute in SSG (#10843)

319bd05

* feat: rerouting in ssg * linting

feat: rerouting

d7f0a47

chore: rename to rewrite

6ad8850

chore: better error message

973ddf6

chore: update the chageset

f34c8ad

Apply suggestions from code review

18804bf

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

chore: update docs based on feedback

9eb5dbc

ematipico force-pushed the feat/reroute branch from 3e4a6a8 to 9eb5dbc Compare May 7, 2024 12:20

lock file

c0149cd

sarah11918 reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

sarah11918 reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

sarah11918 reviewed May 7, 2024

View reviewed changes

.changeset/pink-ligers-share.md Outdated Show resolved Hide resolved

sarah11918 reviewed May 7, 2024

View reviewed changes

.changeset/pink-ligers-share.md Outdated Show resolved Hide resolved

sarah11918 reviewed May 7, 2024

View reviewed changes

.changeset/pink-ligers-share.md Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved

matthewp reviewed May 7, 2024

View reviewed changes

packages/astro/src/core/middleware/callMiddleware.ts Outdated Show resolved Hide resolved

bholmesdev reviewed May 7, 2024

View reviewed changes

ematipico and others added 3 commits May 7, 2024 16:14

Apply suggestions from code review

d3f22ca

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> Co-authored-by: Matthew Phillips <matthew@skypack.dev> Co-authored-by: Ben Holmes <hey@bholmes.dev>

feedback

a63885f

rename

d254dcb

ematipico force-pushed the feat/reroute branch from b4f482e to d254dcb Compare May 7, 2024 15:36

ematipico added 3 commits May 7, 2024 16:48

add tests for 404

78b13f8

revert change

f733520

fix regression

bab2d41

matthewp approved these changes May 7, 2024

View reviewed changes

sarah11918 reviewed May 7, 2024

View reviewed changes

.changeset/pink-ligers-share.md Outdated Show resolved Hide resolved

sarah11918 approved these changes May 7, 2024

View reviewed changes

Update .changeset/pink-ligers-share.md

ff5602a

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

bholmesdev approved these changes May 7, 2024

View reviewed changes

ematipico merged commit 47877a7 into main May 8, 2024
14 checks passed

ematipico deleted the feat/reroute branch May 8, 2024 09:26

astrobot-houston mentioned this pull request May 8, 2024

[ci] release #10944

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: rewriting (experimental) #10867

feat: rewriting (experimental) #10867

ematipico commented Apr 24, 2024

changeset-bot bot commented Apr 24, 2024 •

edited

github-actions bot left a comment

bluwy left a comment

matthewp commented Apr 26, 2024

natemoo-re commented Apr 26, 2024

sarah11918 commented May 1, 2024 •

edited

ematipico commented May 6, 2024

sarah11918 left a comment

sarah11918 May 7, 2024

bholmesdev left a comment

sarah11918 left a comment

feat: rewriting (experimental) #10867

feat: rewriting (experimental) #10867

Conversation

ematipico commented Apr 24, 2024

Changes

Testing

Docs

changeset-bot bot commented Apr 24, 2024 • edited

🦋 Changeset detected

github-actions bot left a comment

Choose a reason for hiding this comment

bluwy left a comment

Choose a reason for hiding this comment

matthewp commented Apr 26, 2024

natemoo-re commented Apr 26, 2024

sarah11918 commented May 1, 2024 • edited

ematipico commented May 6, 2024

sarah11918 left a comment

Choose a reason for hiding this comment

sarah11918 May 7, 2024

Choose a reason for hiding this comment

bholmesdev left a comment

Choose a reason for hiding this comment

sarah11918 left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Apr 24, 2024 •

edited

sarah11918 commented May 1, 2024 •

edited