kit icon indicating copy to clipboard operation
kit copied to clipboard

Logging for overridden config in vite does not include cors options for dev and preview

Open fnimick opened this issue 1 year ago • 7 comments

Describe the bug

Some config options are set by sveltekit and are not warned if they are overriding user-supplied vite options, though other options do have warnings enabled.

For example, the svelte plugin explicitly sets server.cors and preview.cors to { preflightContinue: true }. This fails to report an overridden config if the user configuration was either cors: false or cors: { preflightContinue: false }.

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-l9njmw

This has server.cors set to { preflightContinue: false } and preview.cors set to false. Both are overriden with no warning.

Logs

Resolved config:

  vite:config   server: {
  vite:config     preTransformRequests: true,
  vite:config     cors: { preflightContinue: true },
  vite:config     fs: {
  vite:config       strict: true,
  vite:config       allow: [Array],
  vite:config       deny: [Array],
  vite:config       cachedChecks: undefined
  vite:config     },
  vite:config     sourcemapIgnoreList: [Function: sourcemapIgnoreList],
  vite:config     watch: { ignored: [Array] },
  vite:config     middlewareMode: false
  vite:config   },
  vite:config   preview: {
  vite:config     port: undefined,
  vite:config     strictPort: undefined,
  vite:config     host: undefined,
  vite:config     https: undefined,
  vite:config     open: undefined,
  vite:config     proxy: undefined,
  vite:config     cors: { preflightContinue: true },
  vite:config     headers: undefined
  vite:config   },


### System Info

```Shell
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: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @sveltejs/adapter-auto: ^3.0.0 => 3.2.3 
    @sveltejs/kit: ^2.0.0 => 2.5.21 
    @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.1.1 
    svelte: ^4.2.7 => 4.2.18 
    vite: ^5.0.3 => 5.4.0

Severity

annoyance

Additional Information

No response

fnimick avatar Aug 12 '24 18:08 fnimick

I suspect fixing this may be as simple as adding the preflightContinue keys to the enforced_config at https://github.com/sveltejs/kit/blob/03bc86025643f30132f658ef608c5fe09f6e8234/packages/kit/src/exports/vite/index.js#L41

fnimick avatar Aug 12 '24 20:08 fnimick

~~it looks like that was added in https://github.com/sveltejs/kit/pull/9619/files but not sure if intentional.~~

~~@benmccann do you remember why that cors setting was added to the config modifications?~~

different PR, see below

dominikg avatar Aug 12 '24 20:08 dominikg

That commit appears to be simply moving the cors: { preflightContinue: true } option. The options were added in https://github.com/sveltejs/kit/pull/8731

I'm not suggesting removing these options, but sveltekit overriding user provided options should be included in the vite warning log, similar to other options that are overridden.

I believe preflightContinue is necessary for the vite default cors handler to pass requests through to OPTIONS endpoints in dev and preview.

fnimick avatar Aug 12 '24 21:08 fnimick

I am not sure overriding user provided options is the right way here. having cors disabled is something users might want even in dev/preview. The example in #8731 (good find) indicates that explicit false should be.

cc @eltigerchino

dominikg avatar Aug 12 '24 21:08 dominikg

as a workaround, you can add a vite plugin with a config hook after the sveltekit plugin to reset it to false:

plugins:[sveltekit(),{
  name: 'vite-plugin-no-cors-config',
  config(){
    return {server:{cors: false}, preview:{cors: false}}
  }
}]

dominikg avatar Aug 12 '24 21:08 dominikg

Yeah, I’m highly in favour of letting user config take precedence. I think the only reason I didn’t do so in that PR was because the preflight continue option managed to preserve existing behaviour while allowing the options endpoint to work

teemingc avatar Aug 13 '24 04:08 teemingc

A proposal, re: "letting user config take precedence":

  • if server.cors or preview.cors is unset (undefined): set to { preflightContinue: true }
  • if server.cors or preview.cors is an object, and preflightContinue is unset: set to true
  • if server.cors or preview.cors is true, or server.cors.preflightContinue or preview.cors.preflightContinue is false, do not modify config (and potentially warn in the log that OPTIONS endpoints may not work due to vite handling requests and not passing through to sveltekit)
  • If server.cors or preview.cors is false, do not modify config

Is this behavior too complicated? It seems to me that it would be the best DX where OPTIONS handlers will 'just work' but vite cors handling will not be enabled if the user is explicitly trying to disable it.

fnimick avatar Aug 20 '24 17:08 fnimick