[refactor] create ViteDevServer in dev function instead of in Watcher

[benmccann]

It doesn't really make make sense for the server to be created inside the file system watcher. This makes them both be created in dev. The next step might be to see if we can create the watcher inside the Vite plugin. At that point, SvelteKit dev mode would really be doing nothing except for creating Vite config and plugin.

[changeset-bot]

⚠️ No Changeset found

Latest commit: b20a11d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dominikg]

could we use the watcher exposed by vite in server.watcher , accessible from configureServer hook?

https://vitejs.dev/guide/api-javascript.html#vitedevserver

[Rich-Harris]

Unfortunately this is too far out of date since #2931 and would need to be redone.

I'm not 100% sure what it's buying us, though? The dev service is comprised of two parts — a filewatcher, and a Vite server — and that's true whether they belong to a single class defined in src/core/dev/index.js or two separate objects both of which are returned to the caller in src/cli.js. Personally I prefer to have src/cli.js know as little as possible about the implementation of the various commands.

It doesn't really make make sense for the server to be created inside the file system watcher

This isn't what's happening — the file system watcher and the server are both being created inside the Watcher class; they're siblings. If it's just a case of bad naming then we could call it DevService or something instead?

[Rich-Harris]

could we use the watcher exposed by vite in server.watcher , accessible from configureServer hook?

yeah, that would make sense. I prefer cheap-watch generally, but there probably isn't anything to be gained by using it when there's already a chokidar instance lying around

[Rich-Harris]

Experimenting locally with using the Vite filewatcher. One wrinkle I've found: because Vite's own handlers are invoked first, if you delete a page you'll get an error in the console telling you that an import can't be resolved, because the manifest hasn't yet been updated to remove that import. Not sure if that's something we have any control over?

[Conduitry]

I won't be mortally offended if SvelteKit stops using cheap-watch - it's had way more of an exciting life and way more npm downloads already than I ever expected it to.

There was also an issue #2793 recently about allowing polling instead of filesystem events, and I think that could probably also be resolved if we were using Vite's own copy of Chokidar?

[dominikg]

There was also an issue #2793 recently about allowing polling instead of filesystem events, and I think that could probably also be resolved if we were using Vite's own copy of Chokidar?

yes: https://vitejs.dev/config/#server-watch

example: https://github.com/sveltejs/vite-plugin-svelte/blob/c92f1bf82d5fafa8c08de463879e5d2b2728583a/packages/e2e-tests/hmr/vite.config.js#L20

[Rich-Harris]

because the manifest hasn't yet been updated to remove that import. Not sure if that's something we have any control over?

Actually this seems unrelated — I can create the same error on master. Something to do with the client-side .svelte-kit/dev/generated/manifest.js file not being invalidated correctly

[benmccann]

My first commit on this PR made more sense, but the second one wasn't in a great state and then I had to go to bed. Thanks Rich for taking over on this one. I'll close in favor of #3138