Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Expose kit.vite in svelte.config.cjs #509

Closed
GrygrFlzr opened this issue Mar 13, 2021 · 10 comments
Closed

Proposal: Expose kit.vite in svelte.config.cjs #509

GrygrFlzr opened this issue Mar 13, 2021 · 10 comments

Comments

@GrygrFlzr
Copy link
Member

GrygrFlzr commented Mar 13, 2021

Last proposed API from Rich:

module.exports = {
  kit: {
    vite({ mode }) {
      // mode is something like 'dev', 'build', 'service-worker'
    }
  }
};

Currently, #486 changed vite.config.js and the hardcoded Vite build step in order to better support the serverless platform usecase.

This approach of modifying the Vite config is quite restrictive and fragile:

  • It relies the user to update their vite.config.js whenever we update the template or Vite changes their configuration API
  • It prevents the user from modifying some of the configuration we hardcoded - e.g. someone may want to target esnext instead of es2018
  • It requires updating create-svelte and all the examples we have
Old preset proposal here

A separate approach preact has taken is create a withPreact() function which takes advantage of mergeConfig and UserConfig from vite:

import withPreact from "@preact/preset-vite";

export default withPreact({
  // Your usual vite config
}, {
  // Add your options here
  devtoolsInProd: true
});

Another possible suggestion in the Vite discord involved a factory method:

export default defineConfig([
  usePreact({ devtoolsInProd: true }),
  {
    // Vite Config Options
  },
])

Unlike preact, we currently already have access to svelte-preprocess and the community-made svelte-adders, which are able to effectively add plugins like tailwind and mdsvex by only modifying svelte.config.cjs. However, I can imagine more complex situations where a vite plugin may be desirable. This is also more flexible than the snowpack config extending we did in the past.

Either way, this would allow us to update the config preset alongside kit, instead of having to tell users to migrate their config. This is especially important for the ssr field in the Vite config as the field is marked subject to change in the future.

I'm not entirely sure what the API should look like yet, but the preact examples above should be a good starting point.

Notable things to tackle:

@antony
Copy link
Member

antony commented Mar 13, 2021

Name makes sense to me, since the preact one is prior art.

For the same reason the approach suggested also makes sense.

We get a fair amount of issues raised where people fail to update their config when migrating between versions of libraries - usually because they don't want to read the change log or migration notes.

Anything which reduces this burden therefore is good in my book.

@dominikg
Copy link
Member

would this benefit from utilities provided by vite-plugin-svelte?

Note, in the next release automatic config of svelte depending on vite mode is going to be improved. no more need to specify hot or emitCss

@rixo
Copy link

rixo commented Mar 13, 2021

Yes, yes!

Such a change would also be loved by tools like Svench, that want to programmatically augment / customize the user's config (to be able to build their components with the same plugins, preprocessors, etc., but by changing a few things like port and entry point). With the current Kit implementation, that would imply copying and maintaining the hardcoded config in Kit, with an unpalatable coupling between the tool (eg Svench) and Kit's versions.

@Conduitry
Copy link
Member

I'm also +1 on this. In Sapper, requiring specific code in rollup.config.js/webpack.config.js was always a bit of a pain point, and made it hard to upgrade because of the need to manually merge sapper-template changes with your project's config.

@Rich-Harris
Copy link
Member

At a high level I think this definitely makes sense. I'd suggest maybe @sveltejs/kit/vite-preset rather than a separate package, since the two things are likely to be very tightly coupled in terms of releases (and since it would contain Kit-specific stuff like aliases).

Where I can see it potentially getting tricky is around the service worker build, which needs to have an entirely different config to everything else.

@benmccann
Copy link
Member

I slightly prefer the defineConfig syntax to the withPreact syntax. With withPreact I think it's less clear what goes in the first object vs the second object whereas with defineConfig it's much clearer how those two objects are being used

@Rich-Harris
Copy link
Member

#518 is another example where the shape of the config would potentially need to be somewhat different, which does make me wonder if exposing a preset is the right call versus our existing approach, which is nearly equivalent (except for the order in which config is applied — our config overrides user config, though that's probably for the best since all the options we pass directly from dev and build are necessary and shouldn't be overridden) but gives us more flexibility to vary it for different scenarios.

@Rich-Harris
Copy link
Member

Put another way: a SvelteKit app isn't a Vite app, it's an app that uses Vite under the hood. (You can't run vite or vite build and expect it to do anything good.) So perhaps it would be preferable if most projects didn't have a vite.config.js at all, and either a) it was considered an optional thing for power users, or b) svelte.config.cjs exposed a way to modify the Vite config:

module.exports = {
  kit: {
    vite: ({ config, mode, ssr }) => {
      // mode is something like 'dev', 'build', 'service-worker'
      // config is what we currently pass to Vite programmatically
    }
  }
};

I've come to the view that it would only really make sense to have a preset approach if we rebuilt SvelteKit such that it were possible to do vite or vite build, but I'm not sure that's achievable.

@antony
Copy link
Member

antony commented Mar 14, 2021

it's an app that uses Vite under the hood. (You can't run vite or vite build and expect it to do anything good.)

I hadn't realised that this was the case.

So perhaps it would be preferable if most projects didn't have a vite.config.js at all,

This shouldn't be the preference, it should be the rule. Having a vite.config.js would be hugely confusing, and there's no guarantee that anything you did in there would predictably, if at all. It's misleading.

or b) svelte.config.cjs exposed a way to modify the Vite config

This is what nuxt et al do. It's the right way to do it in my opinion (although when I used nuxt their modifier was all sorts of weirdness and black-box element find-and-merging. The above looks hugely simpler (maybe you could pass the entire config object as the last parameter to that method, for power users.

@Rich-Harris
Copy link
Member

That sounds convincing. Given my earlier comments about the necessity of Kit-provided config not being overridden, perhaps the signature of config.kit.vite should be ({ mode }) => config, i.e. the user-provided function doesn't get to see/edit our config

@GrygrFlzr GrygrFlzr changed the title Proposal: @sveltejs/preset-vite Proposal: Expose kit.vite in svelte.config.cjs Mar 15, 2021
@Rich-Harris Rich-Harris added this to the public beta milestone Mar 18, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants