merge resolve options when using sequence

[Rich-Harris]

Closes #4051.

After this change, a transformPageChunk option passed to the first handle in a sequence will be respected by later calls to resolve

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: c7081b7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

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

[benmccann]

are there docs that should be updated or added to?

[Rich-Harris]

had a crack at that in 3862c84

[benmccann]

How about https://kit.svelte.dev/docs/modules#sveltejs-kit-hooks ? I was guessing it would need to be updated

[dummdidumm]

It feels weird to me that transformPageChunk is not called in order (last is first instead of last). Is there a reason to do it like this?

[Rich-Harris]

Ah, dangit — I thought adding the description to the source code would update the docs, forgot I needed to update ambient.d.ts instead. Would be great if the types came directly from the source code and we didn't need to declare module for @sveltejs/kit/*, not sure what it would look like to set that up.

It feels weird to me that transformPageChunk is not called in order (last is first instead of last). Is there a reason to do it like this?

It's analogous to calling add_a(add_b(add_c('x'))) — the result is xcba not xabc

[dummdidumm]

It's analogous to calling add_a(add_b(add_c('x'))) — the result is xcba not xabc

Yes, but because it's an array I read this left-to-right/top-to-bottom, not the other way around. Is there a situation where switching this behavior is strange/wrong?
I would at least make it more clear in the JSDoc that the last one runs first.

[Rich-Harris]

Is c7081b7 not clear enough? I'm not sure what more we can do.

Is there a situation where switching this behavior is strange/wrong?

I mean... yeah! You'd literally be changing the semantics to xabc. I think it would be really weird if the transformPageChunk that's applied directly to the response coming from SvelteKit ran after more distant transform functions. Think of it like this:

// src/hooks.js
async function increment({ event, resolve }) {
  const response = await resolve(event, {
    transformPageChunk: ({ html }) => {
      return html.replace(/number: (\d+)/g, (_match, n) => `number: ${+n + 1}`);
    }
  });

  const number = response.headers.get('x-number');
  response.headers.set('x-number', String(+number + 1));

  return response;
}

async function double({ event, resolve }) {
  const response = await resolve(event, {
    transformPageChunk: ({ html }) => {
      return html.replace(/number: (\d+)/g, (_match, n) => `number: ${+n * 2}`);
    }
  });

  const number = response.headers.get('x-number');
  response.headers.set('x-number', String(+number * 2));

  return response;
}

export const handle = sequence(increment, double);

// src/routes/+page.js
export function load({ setHeaders }) {
  const number = 100;

  setHeaders({ 'x-number': String(number) });

  return { number };
}

<!-- src/routes/+page.svelte -->
<script>
  export let data;
</script>

<h1>number: {data.number}</h1>

The way it's currently implemented, <h1>count: 100</h1> would be transformed to <h1>count: 201</h1>, and x-count would be transformed from 100 to 201, because double is applied to the response from SvelteKit followed by increment.

If we reversed the order in which transformPageChunk was applied to make it xabc instead of xcba, you'd be left with an incorrect result — the header would say 201 but the HTML would say 202.

Unless, that is, we reversed the order altogether and did this...

const handle = sequence(
  function a({ event, resolve }) {
    console.log('i am called last');
    return resolve(event); // calls SvelteKit
  },
  function b({ event, resolve }) {
    console.log('i am called in the middle');
    return resolve(event); // calls 'a'
  },
  function c({ event, resolve }) {
    console.log('i am called first');
    return resolve(event); // calls 'b'
  }
);

...but that seems deeply unintuitive.

[dummdidumm]

I see - yes this makes sense that way then. Thanks!