[hooks] `sequence` helper function doesn't account for `preload`

[kelvindecosta]

Describe the bug

The sequence helper funciton doesn't seem to call the preload function specified in handle hooks.

Reproduction

Reproduction on StackBlitz

Please find the hooks.server.js file as such:

import { sequence } from '@sveltejs/kit/hooks';

const preloadFonts = async ({ event, resolve }) =>
	await resolve(event, {
		preload: ({ type, path }) => {
			if (type === 'font') {
				console.log(type, path);
				return true;
			}
			return false;
		}
	});

export const handle = sequence(preloadFonts);

// export const handle = preloadFonts;

Run npm run build.

The following log output is seen when handle is set to preloadFonts, but not when it is set to sequence(preloadFonts).

Logs

vite v4.0.0 building SSR bundle for production...
✓ 63 modules transformed.
6:51:28 PM [vite-plugin-svelte] ssr compile done.
package                 files    time      avg
kit-template-default        7   0.14s   20.1ms
.svelte-kit/output/server/vite-manifest.json                    2.71 kB
.svelte-kit/output/server/entries/pages/_page.js                0.05 kB
.svelte-kit/output/server/entries/pages/about/_page.js          0.09 kB
.svelte-kit/output/server/chunks/hooks.server.js                0.27 kB
.svelte-kit/output/server/entries/fallbacks/error.svelte.js     0.62 kB
.svelte-kit/output/server/entries/pages/about/_page.svelte.js   0.93 kB
.svelte-kit/output/server/chunks/stores.js                      1.02 kB
.svelte-kit/output/server/chunks/index2.js                      1.32 kB
.svelte-kit/output/server/chunks/index.js                       4.15 kB
.svelte-kit/output/server/entries/pages/_layout.svelte.js       4.40 kB
.svelte-kit/output/server/entries/pages/_page.svelte.js         7.45 kB
.svelte-kit/output/server/index.js                             80.79 kB
font ./_app/immutable/assets/fira-mono-cyrillic-ext-400-normal-3df7909e.woff2
font ./_app/immutable/assets/fira-mono-all-400-normal-1e3b098c.woff
font ./_app/immutable/assets/fira-mono-cyrillic-400-normal-c7d433fd.woff2
font ./_app/immutable/assets/fira-mono-greek-ext-400-normal-9e2fe623.woff2
font ./_app/immutable/assets/fira-mono-greek-400-normal-a8be01ce.woff2
font ./_app/immutable/assets/fira-mono-latin-ext-400-normal-6bfabd30.woff2
font ./_app/immutable/assets/fira-mono-latin-400-normal-e43b3538.woff2
font ./_app/immutable/assets/fira-mono-cyrillic-ext-400-normal-3df7909e.woff2
font ./_app/immutable/assets/fira-mono-all-400-normal-1e3b098c.woff
font ./_app/immutable/assets/fira-mono-cyrillic-400-normal-c7d433fd.woff2
font ./_app/immutable/assets/fira-mono-greek-ext-400-normal-9e2fe623.woff2
font ./_app/immutable/assets/fira-mono-greek-400-normal-a8be01ce.woff2
font ./_app/immutable/assets/fira-mono-latin-ext-400-normal-6bfabd30.woff2
font ./_app/immutable/assets/fira-mono-latin-400-normal-e43b3538.woff2

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.90 
    @sveltejs/kit: next => 1.0.0-next.579 
    svelte: ^3.54.0 => 3.54.0 
    vite: ^4.0.0 => 4.0.0

Severity

annoyance

Additional Information

No response

[Rich-Harris]

Ah yep. Same is true for filterSerializedResponseHeaders:

https://github.com/sveltejs/kit/blob/1264ae53f8459c9001d6791bf78bed5336146585/packages/kit/src/exports/hooks/sequence.js#L24-L39

[dummdidumm]

Question is which way we apply those, && or ||? In other words, if one handler says we want to preload X and the other one says we want to preload Y, do we preload both, or neither?

[Rich-Harris]

I think the outermost handler (i.e. first to start, last to finish — handle = sequence(outer, middle, inner)) wins. That's how it works in transformPageChunk (slightly different, since they can be chained, but the outer handler can override whatever the inner handler does)

[dummdidumm]

So in my example the outcome would be "only preload X because that was the first handler, and ignore the other handler's preload outputs"?

[Rich-Harris]

exactly, yeah. unless we wanted to do something funky where an undefined doesn't override a boolean, but i think that would be needlessly confusing. in practice, the only handler that is likely to set the preload option is the one you wrote yourself, i.e. here...

export const handle = sequence(
  logger(),
  auth(),
  ({ event, resolve }) => {
    // ...
  },
  something_else()
);

...logger, auth and something_else almost certainly won't be messing with preload, so as long as the option is respected, the precedence rules are probably not all that important

[kelvindecosta]

With regard to preload specifically, is there a need for multiple hooks to each have a preload option?

Assuming that all the "preloadable" files are found at build time, each hook that enables the preload option should have access to this full set of assets.

It makes sense then, to have just one hook that enables preload and the sequence function can be typed such that it warns if there are multiple hooks that are trying to preload.

[arackaf]

Has this been fixed? Not asking in the toxic OSS way but seriously: did someone fix this? I can't repro anymore; preload seems to work fine from within a sequence chain. I'm seeing my fonts correctly preloaded in my page source now when deployed.

[f-elix]

@arackaf Would you mind sharing what your code look like? I've tried again and still can't get it to work with sequence.

[arackaf]

Sure. Here's what mind looks like

import { sequence } from "@sveltejs/kit/hooks";
import { SvelteKitAuth } from "@auth/sveltekit";
import GoogleProvider from "@auth/core/providers/google";
import { GOOGLE_AUTH_CLIENT_ID, GOOGLE_AUTH_SECRET, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, DYNAMO_AUTH_TABLE } from "$env/static/private";

import { DynamoDB, type DynamoDBClientConfig } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocument } from "@aws-sdk/lib-dynamodb";
import { DynamoDBAdapter } from "@next-auth/dynamodb-adapter";
import { getUserSync } from "$data/legacyUser";

const auth = SvelteKitAuth({
  providers: [
    // @ts-ignore
    GoogleProvider({
      clientId: GOOGLE_AUTH_CLIENT_ID,
      clientSecret: GOOGLE_AUTH_SECRET
    })
  ],
  session: {
    maxAge: 60 * 60 * 24 * 365,
    strategy: "jwt"
  },

  // ...
  callbacks: {
    // ...
  }
});

const PRELOAD = new Set(["font", "js", "css"]);

async function handleFn({ event, resolve }: any) {
  const response = await resolve(event, {
    preload: ({ type }: any) => PRELOAD.has(type)
  });

  return response;
}

export const handle = sequence(handleFn, auth);

[f-elix]

Thanks! This is pretty much what I have as well. Do you see anything if you console.log something inside of preload?

[arackaf]

Wow! This is weird.

Locally, when I do npm run build and npm run preview I do NOT see any console.log's in my preload, and I do NOT see my font files being preloaded.

But when deployed on Vercel I DO see my font files preloaded (did not go through the trouble of deploying those console.log statements, and then digging up Vercel's logs).

@Rich-Harris are asset preloads handled differently by default on Vercel, vs a non-Vercel running of a prod build? I can't imagine that they would, but somehow that's what I'm seeing

[f-elix]

I checked my Vercel function logs with the console.log deployed and I don't see it, and the font links aren't there as well. I'm not sure why you see your font preload links when deployed, but I don't believe this issue has been fixed yet.

[allezxandre]

[...] in practice, the only handler that is likely to set the preload option is the one you wrote yourself, i.e. here...

export const handle = sequence(
  logger(),
  auth(),
  ({ event, resolve }) => {
    // ...
  },
  something_else()
);

...logger, auth and something_else almost certainly won't be messing with preload, so as long as the option is respected, the precedence rules are probably not all that important

I would argue that a sensible behaviour for chaining multiple preloads would be to OR the result, like a set, on the basis that each handler knows what should be preloaded, but not what shouldn't. (Such a behaviour is undoubtedly opinionated and would certainly have to be documented)

So in your example, while the last handler will likely know whether it wants to preload based on the type, auth could add to it that it needs to preload some specific asset like chunk/public-key.js, while another imaginary locale() handler could add chunk/translations-es.js, regardless of whether or not the last handler wants to include js assets.

[half2me]

I have a similar issue, also using Auth.js: https://github.com/sveltejs/kit/discussions/7027#discussioncomment-5094578

[Renkas]

I have a similar issue, also using Auth.js: #7027 (reply in thread)

I'm having exact same issue and I can't find a workaround for this.

[kvetoslavnovak]

Hi, any progress with this issue? I am using Supabase SvelteKit Auth helpers and they also warn us regarding this issue with filterSerializedResponseHeaders not working when using sequence.

[smukkejohan]

Any progress or workarounds for this yet? Im trying to implement an api with x-total-count headers but also use hooks for authentication and cors thus need to use it with sequence.

[ErcouldnT]

Still looking for workarounds to use Supabase SvelteKit Auth helpers with sequence.

[Renkas]

Is this PR trying to fix this issue? https://github.com/sveltejs/kit/pull/9739