kit icon indicating copy to clipboard operation
kit copied to clipboard

Add support for per-directory `+hooks.server.js` route files

Open accuser opened this issue 3 years ago • 14 comments

Describe the problem

The current hooks functionality is really useful, particularly when used with sequence() to create an event pipeline. However, I'm increasingly finding that I want to specialise hooks for a given request paths, which requires pathname checking. Whilst this works, it is brittle - changing the route folder requires changing the pathname check. It would be preferable to colocate specialised hooks with the routes they pertain to.

Describe the proposed solution

I would like to be able to add a +hooks.server.js (or .ts) route file to a route folder that would export server hooks specifically for that route and its children. The request event handler would call the exported handle function after the current hooks.server handle function had been called, effectively appending it to the global hook sequence.

Where a child route folder has parent route folders with hooks, these would be appended prior to any hooks specific to the child.

Alternatives considered

Prior to the introduction of generated types I had used an interceptor pattern to wrap Handle and / or RequestHandler, but this has become too unwieldy to type correctly, and has resulted in unnecessary boilerplate-like code.

Importance

would make my life easier

Additional Information

No response

accuser avatar Sep 11 '22 08:09 accuser

This might be a good way to handle the problem described in https://github.com/sveltejs/kit/issues/6315

cdcarson avatar Sep 11 '22 13:09 cdcarson

It would be helpful to have some concrete use cases for this. We've discussed it a fair amount but so far determined that it wouldn't be a good addition, because it would add a lot more complexity.

For example, if you have a situation like this...

src/
├ routes/
│ ├ foo/
│ │ ├ bar/
│ │ │ ├ baz/
│ │ │ │ ├ +layout.js
│ │ │ │ │ +page.js
│ │ │ │ └ +page.svelte
│ │ │ └ +layout.js
│ │ └ +layout.js
│ └ +layout.js
└ hooks.server.js

...then there's some stuff you have to understand about what happens when, but it basically makes sense:

  • handle in src/hooks.server.js starts
    • load functions in the +layout.js and +page.js files run concurrently
    • once load functions finish, we render +page.svelte
  • handle finishes

If we now add a bunch of +hooks.server.js files...

src/
├ routes/
│ ├ foo/
│ │ ├ bar/
│ │ │ ├ baz/
+ │ │ │ ├ +hooks.server.js
│ │ │ │ ├ +layout.js
│ │ │ │ ├ +page.js
│ │ │ │ └ +page.svelte
+ │ │ ├ +hooks.server.js
│ │ │ └ +layout.js
+ │ ├ +hooks.server.js
│ │ └ +layout.js
+ ├ +hooks.server.js
│ └ +layout.js
└ hooks.server.js

...then the order gets somewhat more complex:

  • handle in src/hooks.server.js starts
    • handle in src/routes/+hooks.server.js starts
      • handle in src/routes/foo/+hooks.server.js starts
        • handle in src/routes/foo/bar/+hooks.server.js starts
          • handle in src/routes/foo/bar/baz/+hooks.server.js starts
            • load functions in the +layout.js and +page.js files run concurrently
            • once load functions finish, we render +page.svelte
          • handle in src/routes/foo/bar/baz/+hooks.server.js finishes
        • handle in src/routes/foo/bar/+hooks.server.js finishes
      • handle in src/routes/foo/+hooks.server.js finishes
    • handle in src/routes/+hooks.server.js finishes
  • handle in src/hooks.server.js finishes

I feel like the mixture of sequential and concurrent code would be a real source of confusion for people, particularly the fact that e.g. src/routes/foo/bar/+hooks.server.js runs before src/routes/+layout.js (and also after it, sort of). The waterfall is an opportunity for slowness to creep in. And the flip side of being able to scope handle code with more granularity is that it becomes harder to figure out where a particular thing is happening.

On top of that, while it makes sense to chain handle functions, it probably doesn't make sense to chain handleError. Meanwhile handleFetch could probably go either way. These sorts of ambiguities are best avoided unless they're absolutely necessary.

Rich-Harris avatar Sep 20 '22 00:09 Rich-Harris

Thank you for taking time to consider this @Rich-Harris.

Authorisation is my primary use case for adding optional +hooks.server.js to routes. I want to check a user's authorised scopes before handling a request, and sometimes this needs to be nested. I was able to do this with a simple interceptor that wrapped the handler, but it wasn't easily portable to the newly typed interfaces. The checks can become boilerplate-like quite quickly, and so pulling them out to a hooks module would be useful.

I've just read the proposal to add guard functions to layouts #6912 and this is similar to what I am looking for, albeit extending the +layout.server.js module.

accuser avatar Sep 20 '22 21:09 accuser

Reflecting on your response - and just thinking about handle - if the hooks in your example were necessary, they would reside with hooks.server.js right now, and would be called sequentially. It would be simpler from the framework's point of view, since they would be exported using sequence() or a single handle implementation. From a developer point of view, this would be more complex.

Also, +hooks.server.js proceeds +layout{.server}.js in lexicographical order, and so I don't think it would be too much of a stretch for developers to understand the order in which the modules are called (I don't think we should name future route files with this in mind, but it is useful in this instance.) While the hook will run before the layout and page load functions, the fact is that is can operate before, after, and around (i.e., before and after) the load. This can afford lifecycle functions to individual routes, and goes a long way to providing a middleware solution for SvelteKit.

The benefit of colocating hooks with the routes that they apply to reduces the overhead of guard functions to check if the hook applies to the request or not. I accept that it would introduce complexity for the framework as it would be necessary to create route specific sequences, but the benefit will be for the developer.

A lot could be achieved by in +layout{.server}.js but this is adding business logic to layout, even if the functions were imported from elsewhere. I do think that a route +hooks.server.js resolves this, and is obviously entirely optional for the developer.

Edit: I should add, I hadn't considered handleError and handleFetch in the original post, and agree that the utility of these is ambiguous right now.

accuser avatar Sep 27 '22 09:09 accuser

I feel like this issue can almost be renamed to "Add support for route-level middleware".

To summarize the two big use cases:

  1. Enabling "route guards" or "protected routes"
  2. Empowering the SvelteKit ecosystem with pluggable route-level middleware
    • Right now only possible through brittle string matching in hooks.*.js

I'm not personally convinced this is actually that complicated to introduce to developers. The mental model could be as simple as:

  • "Before" hooks run first, synchronously
  • Load functions run next, async by default
  • "After" hooks run last, synchronously

Load functions are introduced earlier in the docs and tutorial, encouraging async by default, with hooks beign introduced under the "advanced" section in both the docs and tutorial. You could really hammer the point home with a warning like:

Use +hooks.client.js and +hooks.server.js sparingly for things like authentication where you intentionally want route-level, blocking waterfalls.

As for the question about handle, handleError, handleFetch, they could be simply answered with "they all work the same as with any other middleware". If a route wants to handle requests, errors, or intercept fetches, let it. If a developer wants to selectively opt-in to anything else, they can do so with their own middleware.

Finally, this all could be made a bit more simple in SvelteKit v2 by deprecating src/hooks.*.js in favor of a top-level src/routes/+hooks.*.js. 🙂

dslatkin avatar Oct 20 '23 20:10 dslatkin

I think if people are going to use SK as their backend with a filesystem based router that router regardless of its description needs to support middleware. If I were using express or koa or whatever I would have middleware groups, which solve this problem entirely.

Whether it's called hooks or not, I don't see how a functional router can exist without supporting middleware. If I made an API router that you had to string match the URI to choose to run middleware it would be DOA, I don't see how SK can be a mature full stack solution without this.

endigma avatar Feb 12 '24 14:02 endigma

SvelteKit does now support +hooks.server.js: https://kit.svelte.dev/docs/hooks#server-hooks

I've renamed this issue to clarify that the request here is to be able to include such a file in each routes sub-directory

benmccann avatar Feb 12 '24 16:02 benmccann