Consider how to pass options between handlers in sequence

[f-elix]

Describe the bug

When using the sequence helper to chain multiple handle functions, transformPage is only actually run in the last one. Not sure if it was intended or not, but it would be nice if we could use transformPage in all handles, regardless of order.

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-wg5zme?file=src%2Fhooks.js&terminal=dev

You can see in the reproduction that the html lang attribute is not changed. If we swap the two handle functions so that the one that uses transformPage comes last, the html is changed.

Logs

No response

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
    Memory: 19.88 GB / 39.76 GB
  Binaries:
    Node: 16.13.1 - C:\Program Files\nodejs\node.EXE
    npm: 8.4.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (98.0.1108.43)
    Internet Explorer: 11.0.22000.120
  npmPackages:
    @sveltejs/adapter-vercel: 1.0.0-next.43 => 1.0.0-next.43
    @sveltejs/kit: ^1.0.0-next.278 => 1.0.0-next.278
    svelte: ^3.46.4 => 3.46.4

Severity

serious, but I can work around it

Additional Information

No response

[Rich-Harris]

Yeah, this is a broader issue around passing options between handlers. I think we probably want to change the behaviour of sequence to something like this...

/**
 * @param {...import('types').Handle} handlers
 * @returns {import('types').Handle}
 */
export function sequence(...handlers) {
  const length = handlers.length;
  if (!length) return ({ event, resolve }) => resolve(event);

  return ({ event, resolve }) => {
    return apply_handle(0, event);

    /**
     * @param {number} i
     * @param {import('types').RequestEvent} event
     * @returns {import('types').MaybePromise<Response>}
     */
-    function apply_handle(i, event) {
+    function apply_handle(i, event, options = {}) {
      const handle = handlers[i];

      return handle({
        event,
-        resolve: i < length - 1 ? (event) => apply_handle(i + 1, event) : resolve
+        resolve: i < length - 1 ? (event, options) => apply_handle(i + 1, event, options) : resolve,
+        options
      });
    }
  };
}

...which would change the API to this:

import { sequence } from '@sveltejs/kit/hooks';
 
/** @type {import('@sveltejs/kit').Handle} */
async function first({ event, resolve }) {
  console.log('first pre-processing');
-  const result = await resolve(event);
+  const result = await resolve(event, { transformPage });
  console.log('first post-processing');
  return result;
}
 
/** @type {import('@sveltejs/kit').Handle} */
-async function second({ event, resolve }) {
+async function second({ event, resolve, options }) {
  console.log('second pre-processing');
-  const result = await resolve(event);
+  const result = await resolve(event, options);
  console.log('second post-processing');
  return result;
}
 
export const handle = sequence(first, second);

Another idea we discussed in the maintainers' chat recently was to automatically merge options on the way 'down', but I think I prefer the explicit approach.

[f-elix]

Thanks for the explanation!

My 2 cents:

It seems to me that having to explicitly pass the options to every other handles would become cumbersome if you have a lot of them. Moreover, I also don't think automatically merging the options would remove any flexibility for the user.