feat: add CLI keyboard shortcuts

[ArnaudBarre]

This is based on the work from #9673 by @HomyeeKing , @aleclarson and @joelmukuthu

Closes #9673

Few changes have been made:

• Don't display hint by default, I think it's nice for people building on top of Vite
• Don't display quit in default hint, I think people know how to quit a process by default
• Fix rebind of shortcuts for external restart
• Send SIGTERM for ctrl+c, enable nicer exit and probably better in middleware mode (not tested)
• Restore initial hmrConfig on HMR toggle
• Enable custom shortcuts and use it in cli for stopping dev profiler

[ArnaudBarre]

cc @benmccann, @dominikg: What do you think about this? Is this ok for SvelteKit?

[benmccann]

SvelteKit doesn't really do anything special, but just has users call vite directly, so any change to the CLI should generally be fine as long as our Playwright tests can still start and stop processes just the same. Since Vite also uses Playwright that's probably pretty safe, but you could invoke it against the ecosystem CI if you really wanted to be sure.

[patak-cat]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
astro	✅ success
histoire	✅ success
iles	✅ success
ladle	✅ success
laravel	✅ success
marko	✅ success
nuxt-framework	❌ failure
rakkas	⏹️ cancelled
sveltekit	✅ success
vite-plugin-ssr	❌ failure
vite-plugin-react	✅ success
vite-plugin-react-swc	✅ success
vite-plugin-svelte	✅ success
vite-plugin-vue	✅ success
vite-setup-catalogue	❌ failure
vitepress	✅ success
vitest	❌ failure
windicss	✅ success

[dimshik100]

This patch appears to add keyboard shortcut support to the Vite CLI. The main changes are the addition of a new file, shortcuts.ts, which exports a bindShortcuts function that binds the specified shortcuts to the CLI, and updates to the cli.ts file that use this function to bind the default set of shortcuts as well as an additional shortcut to stop the profiler, if it is running.

One issue with this patch is that it uses the global object to store the Session object used by the profiler. This is not recommended, as it can lead to variable name collisions and other issues if multiple modules use the global object in this way. It would be better to pass the Session object as an argument to the stopProfiler function, rather than using the global object.

Another issue is that the stopProfiler function takes a log function as an argument, but the implementation of the shortcut that calls this function does not pass a function. Instead, it passes the result of calling the info method on the server.config.logger object, which is not a function. This will cause an error at runtime. To fix this, the info method should be called within the stopProfiler function, rather than passing the method itself.

Aside from these issues, the changes in this patch appear to be well-structured and well-written. Adding keyboard shortcut support is a useful feature, and the implementation looks correct.

[benmccann]

Looks like ChatGPT was 0/2 on its code review suggestions, which are both wrong

[bluwy]

The node docs showed that it's commonly emitted as SIGINT for all platforms. Seems like SIGTERM isn't supported in Windows too. Should we change this to SIGINT?

With that said we only listen for SIGTERM for the server today (not sure if that has been an issue in Windows), but I think it's nice to follow the default.

[ArnaudBarre]

I will test if it's still exit cleanly on mac os with SIGINT

[ArnaudBarre]

It doesn't close at all when emitting SIGINT.

Doing await server.close().finally(() => process.exit()) works fine, but maybe not perfect for middleware mode

[bluwy]

This should be removed with the internal change too.

[ArnaudBarre]

The build process force me to expose it even when it's internal. Do you know how to fix it?

[bluwy]

Ah this issue. Maybe we could refactor a bit to have bindShortcuts() do a server._shortcutsBound = true in the function itself? That way we can remove

vite/packages/vite/src/node/server/index.ts

Lines 448 to 451 in 6ae1d2d

	bindShortcuts(opts = {}) {
	bindShortcuts(server, opts)
	server._shortcutsBound = true
	},

and the types so it doesn't get bundled by api extractor.

Otherwise the other option is to post process and strip it off like

vite/packages/vite/scripts/postPatchTypes.ts

Lines 19 to 20 in 6ae1d2d

	// remove picomatch type import because only the internal property uses it
	const picomatchImport = "import type { Matcher as Matcher_2 } from 'picomatch';"

The first one would be easier and also prevents someone accessing it.

[patak-cat]

You can also hide it in a weak map against the server, no?

[ArnaudBarre]

Found something else, tell me what you think

[ArnaudBarre]

I updated the custom profiler shortcut to enable profiler restart/stop and allow to create profile session for HMR updates

[patak-cat]

Works great! I think we should merge and release it so we can give at least a day to the ecosystem to test it.

@ArnaudBarre if you would like, maybe we could add in another PR a title when pressing h. So it looks like in Vitest

Right now it looks a bit confusing when pressing twice (although it is an edge case)

but the extra line and title help while reading the options. We could also align the options so h and p, etc are all on the same col.

[ArnaudBarre]

Yeah we need to know if ctrl+c works on Windows.

I think that with blank line + title we don't need to align. Will to it in a next PR

[dbousamra]

Hi all. I believe this PR has broken Ctrl-C behaviour somehow. I notice if I run 2 vites servers using tools like yarn workspace foreach or even just npm-run-all where I run vite with another command, vite will not shutdown with ctrl-c.

[patak-cat]

@dbousamra I see you also found #11563, a fix on top of this PR. If you have a diff repro from #11434 or you are seeing a regression from it, please create an issue with a new repro against the latest main so we can track it. A comment in an old PR would get lost quickly.

[dbousamra]

@patak-dev False alarm. Was actually an issue with vitest, not yarn. Was very confused