-
-
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
feat: add server middleware support #13430
Conversation
|
Could this somehow be leveraged to bypass CSRF protection for individual routes? See #6784 |
Does the middleware run in both client and server? Or server only? |
This is a server-only concept, clarified in the PR description |
What's the difference between this new middleware API or using more specific hooks like |
This seems to include GET requests for routes that doesn't include a server load fn and the API is limited. |
This is a layer that allows us to take advantage of technology like Vercel's Edge Middleware, Netlify's Edge Functions, etc. to put a consistent layer of logic between the request and your app, no matter what kind of rendering you're using. Paired with the new serverside route resolution, this makes it possible to actually run something on every request to your app (including requests for static assets, prerendered routes, SSRed routes, and client-side navigations), not just those that hit the server. Right now, if you wanted to run serverside logic prior to, say, retrieving a static asset, you could not -- meaning that if you wanted to generate two static pages, Whether you can use this is platform-dependent -- the platform you're using has to actually provide a layer that can run before all requests to your app -- but enough platforms have mechanisms like this that we think it's probably a worthwhile abstraction. |
@elliott-with-the-longest-name-on-github the PR description says server requests? So just seeking explicit clarification that this will also run before client side navigation ofc I guess Simon would have to make that clarification if unknown by you. Thank you for the great detailed response! |
@Antonio-Bennett yes* We recently created |
packages/adapter-vercel/index.js
Outdated
); | ||
|
||
static_config.routes.push({ | ||
src: '/.*', // TODO allow customization? |
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.
I'm pretty much sure we have to ship matchers or some other way to customize this if we want any chance of ent customers using it
@@ -316,6 +316,53 @@ export const transport = { | |||
}; | |||
``` | |||
|
|||
## Middleware hooks | |||
|
|||
The following can be added to `src/hooks.middleware.js`. |
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.
Kinda feel like we should move to src/hooks/middleware.js
so that in 3.0 we could potentially move to src/hooks/handle.js
, src/hooks/reroute.js
, etc. -- would make selectively deploying a hook to edge easier... 🤔
|
||
resolveId(id) { | ||
if ( | ||
id.startsWith('$env/') || |
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.
er, why? environment variables should be available 🤔
I know you just came here to talk shit, but
|
But you can implement middleware support however you want on adapter level. It can run on the edge, it can run wherever you want. Also worth pointing out - Vercel already doesn’t run middleware in the edge environment and they use their new “fluid” stuff. |
Could you elaborate further? Assuming you would have a way to know "an error was thrown" within
It indeed seems like the term "middleware" sparks wildly different thoughts on what this actually is - it's a very overloaded term. We're already thinking about whether or not there's a better term. |
After exploring this we concluded that it's better to leverage the unique capabilities of each deployment provider's platform instead of providing the lowest common denominator.
|
WIP
This adds support for server middleware to SvelteKit.
Middleware will run prior to all requests made to the server, including those to prerendered pages but excluding those to immutable assets. The current state of the API looks like this:
Adapters have to opt in to supporting middleware, and you'll get an error at dev/build time when using it with an adapter that does not support it. Since middleware is very specific to deployment platforms and should be very fast, it deliberately only supports only a limited set of things you can do.
If your app is not using prerendered (or in case of Vercel ISR'd) pages, it probably does make more sense for you to express the desired logic within
handle
andreroute
hooks.TODOs:
redirect/error
in here, and to align the API makereroute
alsothrow
Response
object from a route resolution request - right now the client will just error in the console. We either need to disallow that somehow, or turn the response into some kind of hard reloadpaths.relative: false
when they use middlewarecloses #10870
Please 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