-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: generate edge middleware to run reroute
#12296
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 9fcdfc1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Should this be a bit more generic so that it can be used by other adapters too? or is the way route splitting works so adapter-specific that they all need a separate implementation? |
I think it's possible for the part where we need to import the reroute hook and get the rewritten URL. The rest seems more platform specific. Vercel uses a custom header in a CC: @LorisSigrist |
Each Serverless provider is going to have a different way of rewriting the URL so that would certainly be adapter-specific. Given that There is some fragility since we're assuming that the bundled file will always be at that path, meaning that changes in sveltekit's build-output may break the adapters, but I don't think that warrants adding a shared implementation right now. We can always share it later if the need arises. |
The most straight forward solution here would probably be to have a
Let's wait on this one for now, this can be added later if the need arises from other adapters. When I suggested that we might need to extend the adapter API I didn't know we could just access files from the output-directory directly. |
added a naive implementation to support Netlify then realised the adapter only supports |
I wish we could do this but I'm not sure if it's possible on Netlify which only takes a EDIT: maybe we can add the header in the serverless functions if split is enabled |
That would be ideal As a fallback we could go with a searchParam instead of a header, that should work everywhere. |
what is the worst that would happen? that reroute is called again when the rerouted request hits the function? Implementing it as a query param wouldn't really work unless the param is stripped again before it ever reaches the user. We must not mess with the presence or order in the querystring in any way, otherwise its going to break existing or future apps. Even a custom header could be problematic in cases where headers are validated, but that also requires a change to the reroute signature. To introduce this in a non-breaking way would mean that you'll have to use return type like |
Dominik is right We have an error in our thinking. @eltigerchino and I assumed that the second |
Updated the PR to include the same reroute fixes from #13092 in the Vercel and Netlify middleware functions |
There's currently an issue with Vercel rewrites discarding the SvelteKit form action in the URL because the query parameter doesn't have a value vercel/vercel#12902 |
fixes #11879
This PR adds an edge middleware that runs
reroute
beforehand so that, when there are multiple functions, the correct one is invoked instead of not matching any function matchers.The Vercel middleware uses the rewrite helper from
@vercel/edge
.rewrite
docs - https://vercel.com/docs/functions/edge-middleware/middleware-api#rewritesrewrite
implementation - https://github.com/vercel/vercel/blob/cefda60a603d60cc35e4697c36e751cca411e6bb/packages/functions/src/middleware.ts#L101The Netlify middleware simply requires returning the new URL.
TODO
- [ ] don't runreroute
again in the split function that comes after the middlewarePlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits