Instead of having a specific config option to disable HTML middleware, would it be better to have an option to disable a list of middleware? This way, users could have more flexibility to config the server builtin middleware. something like:

// config
server: {
  disabledMiddlewares: ['html', 'fallback']
}

// server/index.ts
if (!serverConfig.disabledMiddlewares.includes('html')) {
  middlewares.use(indexHtmlMiddleware(server))
}

Maybe not a bad idea if middlewares had names, but we'd basically be having to create a string for each ourselves used just for this purpose.

I also think it'd be harder to use. E.g. in this case you'd have to disable three different middlewares and the only way you'd know which to disable would be to open up the Vite source code vs just setting a single documented option.

Finally, it'd make merging options harder. SvelteKit defaults some Vite options and we'd want to set this one so that users don't need to set it to make their apps work. That's relatively easy with a boolean, but can get more complicated to merge user-supplied options with our defaults when the options are more complex data structures

Hi @benmccann , I read the vite server source code again today, I just realized you don't really need to worry about the HTML middleware if your middleware ends the request early.
https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/index.ts#L529 from here you can see user middleware gets handle the request before the html middleware.

I just checked your svelteKit PR, looks like your middleware does end the request in all cases, and never call the next() function. https://github.com/sveltejs/kit/pull/2232/files#diff-ed48923f0655f935bee505cdcbf40212129ff49a89c2c9d752bd63aa1e6282c4R497
Therefore, this PR prob not needed for sveltekit 1.0

I think you tagged the wrong Ben 😄 But this actually is needed because the SvelteKit middleware must be added after the Vite middleware. Vite must be first because it handles the static assets. Then for requests that aren't for static assets it can fallthrough to the SvelteKit middleware. If I put SvelteKit first then no static asset would ever be served because SvelteKit never calls next() as you point out

Hi @benmccann , the static assets middleware is actually pushed to the stack before user configured middlewares from the code I saw. Please give it a try, let me know if that doesn't work for you.

I did try it. It doesn't work

@benmccann sorry, my bad, looks like the spa fallback middleware should be installed after post hooks. Let me see if I can find the reason why it's before post hook.

@benmccann I just tested at my local. If I move the spa fallback middleware after the postHooks calls, everything works as expected.

I also checked the git history, seems the posthooks always run after the fallback middleware. This actually does not make sense to me, since this is also part of vite html serve logic which should go after post hooks? @yyx990803 sorry to ping you, but do you remember the reason behind this? is it OK for us to move the spa-fallback middleware after post hooks?

good idea. closing in favor of #4670

that did not work (#4670 (comment)), so reopening this PR instead

@benmccann we discussed this today. In principle, this is a valid request. But it is not yet clear if adding a single boolean option is the best here. In case it is boolean, it could be named server.serveHtml but it may be more flexible to allow disabling groups or individual middleware with something like server.middlewares: { html: false }. A review of current SSR playgrounds could help to better identify what is best, as the servers could be potentially simplified with this option.

Another option could be to identify and remove this middleware from the array. The internal middlewares are currently named function

Thanks for taking a look at this @patak-js. I really appreciate you driving all these PRs to completion

The other alternative for implementing this that you mentioned is actually one we'd considered above. I tend to think it's not the best way to do it for the reasons mentioned in this comment: #4640 (comment). I'm happy to reconsider if you all have other thoughts about it

Closing this since it seems to have gotten hung up and there's more important fixes to get in