Ability to override load or use before/after hooks

[benmccann]

One of the most common requests in Sapper is a way to alter fetch or otherwise affect the load calls

Is your feature request related to a problem? Please describe.

• Distributed tracing - add a header to every request to trace requests from frontend all the way to datastore (requested by @adamb on Discord. difficulty setting up honeycomb.io also mentioned in Feature request: Preload argument for context sapper#751)
• Using an internal hostname on the server-side (that's what I want this for. my api server is written in another language)
• Using fetch in setup - e.g. to setup Apollo client (Application initialization - making fetch available outside preload sapper#1243 Feature request: Preload argument for context sapper#751)
• Specifying a base URL (Base url for server fetch requests sapper#489)
• Using a UNIX socket for AWS Lambda (Enable sapper to run on AWS Lambda sapper#356)
• Ability to wrap fetch (Export and cross-fetch don't play nice together sapper#908)
• It currently requests only over HTTP. A user on Discord reported this wouldn't work with his server than listens only on port 443
• Accessing Polka request on server (Pass polka.Request to preload() sapper#1436)

Describe the solution you'd like

I originally sent sveltejs/sapper#1336 though it might not solve all requests. E.g. there was a request to time how long load takes and I don't think my solution would have made that easy to do, so it should probably be updated a bit to account for that.

How important is this feature to you?

This enables a large number of use cases for advanced users, so is fairly important.

[Rich-Harris]

Can these cases be accommodated with loader context? i.e.

<!-- routes/$layout.svelte -->
<script context="module">
  export async function load({ fetch }) {
    return {
      context: {
        special_fetch = make_special(fetch);
      }
    };
  }
</script>

<!-- everywhere else -->
<script context="module">
  export async function load({ context }) {
    const stuff = await context.special_fetch('/stuff.json');
    // ...
  }
</script>

[benmccann]

That's basically what I had originally proposed in sveltejs/sapper#1336, but @ajbouh said that something he wanted to do was collect information about load to send to an external monitoring service (e.g. how long it took to run, how often it failed, etc.), which seems like a reasonable use case. I think we'd have to tweak the API a bit to allow for that.

In his fork he created a way to provide a function that would call load so that he could do his own logic before and after sveltejs/sapper#1336 (comment)

[Conduitry]

Is the desire to override the built-in load in some way? If not, it seems like wrapping it in some other function and sticking that on the context seems like it would work for that as well.

[benmccann]

No, wrapping would be sufficient (there's an example of that in sveltejs/sapper#1336 (comment))

[dummdidumm]

I think there's a difference between "add something else to the context and use that" and "wrap the core function". The latter allows for modifications in one place without affecting other places. Imagine you want to alter fetch behavior three months in, you certainly don't want to change all occurrences of fetch-usage then.

[Conduitry]

Alternatively: Would it be confusing if you could alter core functionality like this, without there being any indication that anything special is happening in any of the page components?

[dummdidumm]

That depends on the user I would say. Having the possibility to do so of course means people could do crazy stuff, and if the functionality exists there should be a big disclaimer like "if you don't want to change the behavior for your whole app, use context instead". But not having the possibility would hurt others. I see it as a kind of a plugin or middleware with the same pros and cons. Pro: you can hide stuff and make everyone use it. Con: That stuff may be too hidden.

[benmccann]

I don't know if we need multiple ways of doing things. That's more for us to create/maintain and potential confusion for users as to how to accomplish something and where to look to see if any extra behavior is happening. I think if we provided the ability to wrap then we wouldn't likely need to provide the ability to edit context in a page as well

[Rich-Harris]

I think if we provided the ability to wrap

doesn't that exist already?

<script context="module">
  import { patch } from 'my-svelte-kit-telemetry-lib';

  export const load = patch(({ fetch }) => {
    // tada
  });
</script>

[benmccann]

The idea is to provide a way to configure it in a single place so that you don't have to update every single template and worry about whether you might miss one

[Conduitry]

That's what I understood Rich's first example to be for. Assuming you point to the loader context on each page, you can later change that in just one place and it will affect every component. I think this seems like a reasonable way to handle this.

Loader context is a more powerful tool than just being able to override specific preexisting loader functions. I agree that we don't want two ways to do this - and if we are just picking one, then it should be the loader context - which also already exists.

[benmccann]

Loader context doesn't seem like it solves the timing / metrics gathering use case. If you want to instrument how long your load calls take I'm not sure how you would do that using only loader context. If we want to punt on that I'm okay with that because loader context will solve the rest of the use cases. I just think that we need to be aware that we might need to add some type of additional instrumentation hooks in the future if we go with loader context now.

If we decide to go the loader context route - I didn't like that in Rich's example that I'd have to change every call to fetch to special_fetch throughout the codebase. I would prefer that SvelteKit's implementation of fetch is provided in the context so that I can just override it with my own version

[Rich-Harris]

My general feeling towards this sort of thing is that if it's possible to satisfy the identified use cases without introducing new API (which as far as I can tell it is, with a combination of context and export const load = patch(({ fetch }) => {...})) then we're better off leaving things as they are and seeing how big a gap it is in practice.

API surface area is expensive; the bar is high. It might well be the case that we do need to add a means of wrapping load calls somehow, but such a thing adds implementation complexity, maintenance burden, documentation requirements, and a layer of indirection that makes the system as a whole more difficult for app developers to understand. So we should leave it out unless and until there's significant demand for it.

[Rich-Harris]

Going to close this as there isn't a clear course of action to take — we can revisit it if the existing APIs prove insufficient in practice

[benmccann]

Reopening this as @ajbouh is reporting a gap in the APIs:

You can't wrap fetch (or otherwise augment the preload state) without SvelteKit platform support because (on the server-side) the information you need to use while wrapping is hidden in the request headers.

So the minimum needed to "get around this" is to have in the same place:

• access to request headers and
• access to the values passed to load/prefetch

[Rich-Harris]

The request headers are available in setup. Is that insufficient?

[benmccann]

@ajbouh I'll leave that question to you (just FYI, if we don't hear anything after awhile we'll probably go ahead and close this)

[ajbouh]

I don't fully understand the data flow between setup and load, but maybe?

Are there values that the server-side invocation of setup can return that go to every server-side load, but never to the client-side one?

We can't use session to pass through a custom fetch, since session gets serialized automatically for the client. Maybe we can use context for that? I'm not really sure what the rules for context are or how brittle using that instead would be.

[Rich-Harris]

Fair warning: some of the nomenclature will probably change, because we have too many things called 'context'.

You can derive a request context in getContext, and derive a session from that in getSession. Then, in your root $layout.svelte you can create a wrapped fetch using the contents of session and make it available to every load function via the load context. IOW request context -> session -> load context.

// src/hooks.js
export function getContext({ headers }) {
  return {...};
}

export function getSession({ context }} {
  return {...};
}

// src/routes/$layout.svelte
export async function load({ fetch, session }) {
  const context = {
    fetch: wrap(fetch, session)
  };

  return { context };
}

[ajbouh]

I see. This flow seems to make a few assumptions about non-global state used by the load function:

1. it must be serializable
2. it is either the same value for the client and the server or the server value must be something we are willing to disclose to clients

In my experience there are a variety of scenarios where we want to use non-serializable state that differs between server-side and client-side code.

Simple counter examples:

1. a specific or cache or logger instance
2. where to submit request events: a publicly accessible endpoint, a private log ingestion service, or just stdout?

[Rich-Harris]

Still not really sure I understand the problem. The wrap(fetch, session) example above could return a different value based on whether it's running in the browser. On the server it could e.g. use environment variables that aren't available in the client. Can you give a concrete example of a problem you can't solve with the current tools?

[ajbouh]

My server-side code primarily runs within Lambda@Edge, where environment variables are not available.

Instead on boot up my server-side pulls its configuration from AWS SSM Parameter Store. (I believe Cloudflare workers are similarly expected to store their configuration in the Cloudflare Worker KV store.) Unlike environment variables, values pulled from these places are subject to expiration and may need to be periodically updated.

In my sapper fork, I initialize a telemetry object from these values and call setContext('telemetry', telemetry) before any route processing happens.

I need this telemetry object for a number of things:

• for direct use in both <script> and <script context="module">
• to properly wrap fetch

Not only is the configuration of this telemetry object completely different between client and server, but the server-side implementation uses an entirely different set of imported dependencies from the client-side.

It is very straightforward to handle both the lack of environment variables and the differing imports when the server-side "context" and client-side "context" have different initialization code paths.

[Rich-Harris]

It is very straightforward to handle both the lack of environment variables and the differing imports when the server-side "context" and client-side "context" have different initialization code paths.

this is where the overuse of "context" gets confusing — do you mean component context (as in getContext and setContext), request context, or load context?

I'm guessing the latter since component context isn't available in <script context="module"> and request.context doesn't exist client-side. Given that, can't you use different initialization code paths like so?

// src/routes/$layout.svelte
import { browser } from '$app/env';

export async function load() {
  const context = browser
    ? await import('$lib/client/context.js')
    : await import('$lib/server/context.js');

  return { context };
}

[benmccann]

I'm going to close this since it's a long thread and no response has been received. If there's a further need it might be best to file a new issue discussing the specific problem that needs to be solved