feat(server): add shortcuts for restart server, open browser, & clear cache #3182

HomyeeKing · 2021-04-28T02:43:31Z

Description

make devServer restartable , convenient to debug

Additional context

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

packages/vite/src/node/cli.ts

aleclarson · 2021-05-28T16:28:25Z

@Shinigami92 @antfu How's this for the output printed on startup?


  vite v2.3.2 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 593ms.

  Available commands:
    Press "r" to restart the dev server.
    Press "f" to rebuild the optimized dependency cache.
    Press "o" to open the browser.

Shinigami92 · 2021-05-28T16:57:40Z

@Shinigami92 @antfu How's this for the output printed on startup?


  vite v2.3.2 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 593ms.

  Available commands:
    Press "r" to restart the dev server.
    Press "f" to rebuild the optimized dependency cache.
    Press "o" to open the browser.

@aleclarson Your vite is outdated and slow 🤣
But yes, looking good

HomyeeKing · 2021-05-28T17:48:42Z

another question:

hide input console ?
auto linebreak ?
using enum to express key binding?

antfu · 2021-05-29T03:08:56Z

About the CLI outputs, I am thinking something like

  vite v2.3.2 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose
  > Shortcuts: "r" restart, "o" open browser, "f" force restart

  ready in 593ms.

packages/vite/src/node/cli.ts

aleclarson · 2021-05-30T14:49:25Z

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

@@ -615,6 +615,8 @@ async function startServer(
      )

      printServerUrls(hostname, protocol, port, base, info)
+      // print shortcuts info
+      info(`  > Shortcuts: "r" restart, "o" open browser, "f" force restart`)


@antfu "force restart" seems too vague? Should we explain it in the docs?

Yeah, think we should document it, as we could possibly have more shortcuts in the future but not necessary to be print out here.

packages/vite/src/node/cli.ts

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

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

packages/vite/src/node/cli.ts

Shinigami92 · 2021-06-01T05:17:01Z

Seems after build the package.json couldn't be found/resolved at runtime anymore 🤔
When watching into the changes after last green tick, I assume it has something to do with function scope or so
Hope this helps to find the error

dominikg · 2021-06-25T09:26:22Z

Should this handle gracefully closing the old server instance and websocket and reconnecting to connected clients? The clients may also need a full reload after the server was restarted as there is no guarantee that the loaded modules still match the state on the new server instance.

patak-dev · 2021-07-04T06:32:19Z

@HomyeeKing could you resolve the conflicts? Tests will be re-run and you can check then if the failure was a glitch.

HomyeeKing · 2021-07-26T06:43:10Z

@tjk sorry for not understand you until now, because I never use in middlewareMode and shortcuts seem usefule in this case

btw, looks like the config never used on middlewareMode, like printUrl, open in browser something
we can do it all by shortcuts now I guess

dominikg · 2021-07-26T08:06:22Z

Hmm, in middleware mode the process is not created by vite cli and you cannot assume that it is safe to hijack the stream.
This would be a breaking change if it defaulted to true, so the default should be false.

but i'm not convinced this is even useful in middleware mode as most of the time the external process would need to handle restart and clearcache additionally to avoid inconsistent state.
cc @brillout, @frandiox, @GrygrFlzr , what do you think?

In addition i think it could be useful to expose restart on the devserver instance so plugins can trigger it in certain situations.
Currently this is possible in a rather hacky way:
see @antfu 's vite-plugin-restart or how it is done in vite-plugin-svelte

antfu · 2021-07-26T08:20:53Z

As we want to keep our configure options minimal as possible, instead of having an option, I would think it might be better to just disable in middleware mode automatically without configurations.

In addition i think it could be useful to expose restart on the devserver instance so plugins can trigger it in certain situations.

I like this idea.

brillout · 2021-07-26T10:08:46Z

I agree with @dominikg: AFAICT I don't see a use case for this for middleware mode.

Actually, in general, why would the user want to restart the dev server? Doesn't HMR handle all code update scenarios?

frandiox · 2021-07-26T10:39:14Z

I'm not using middleware mode for anything yet so I don't have a strong opinion.
Perhaps instead of having an actual option with docs etc, it would be enough to make sure the bindShortcuts function is exported publicly so any developer can use it optionally? bindShortcuts(myServer) or anything like that.

This reverts commit a7dc987.

This reverts commit 100c4e5.

aleclarson · 2021-07-26T17:14:36Z

Actually, in general, why would the user want to restart the dev server? Doesn't HMR handle all code update scenarios?

@brillout It's true that vite.config.js changes already restart the server without this PR. I think manual restart is meant for plugin developers, though @HomyeeKing might have another use case in mind as well.

But honestly, it's probably better for plugin developers to use an external file-watching process that automatically reruns vite command when any file in the plugin directory changes. So there may not be a solid enough use case to warrant a "restart server" shortcut, unless I'm missing something.

In addition i think it could be useful to expose restart on the devserver instance so plugins can trigger it in certain situations.

This idea could probably have its own PR, but I approve

As we want to keep our configure options minimal as possible, instead of having an option, I would think it might be better to just disable in middleware mode automatically without configurations.

@antfu I agree that middleware mode should not have shortcuts bound. Nonetheless, the server.bindShortcuts option is still necessary if we keep the bindShortcuts call within server.listen (which I think is wise, because that's where the logging needs to happen, and we don't want to log the shortcuts if they're not bound).

Since we only bind shortcuts in the `server.listen` method, we can safely listen for the httpServer "close" event to unbind them. This avoids a memory leak.

aleclarson · 2021-07-26T17:21:33Z

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

-  stdin.setEncoding('utf8')
-  stdin.setRawMode(true)
-  stdin.on('data', (data) => {
-    const input = data.toString().trim().toLowerCase()


removed toString call, since setEncoding ensures data is a string

removed trim call, since whitespace is not a concern

removed toLowerCase call, since we may want to use the uppercase shortcuts in the future

aleclarson · 2021-07-26T17:22:32Z

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

@@ -2,30 +2,33 @@ import type { ViteDevServer } from '..'
 import { openBrowser, resolveBrowserUrl } from './openBrowser'
 import { restartServer } from './hmr'

-export function bindShortcuts(server: ViteDevServer, isRestart = false): void {
+export function bindShortcuts(server: ViteDevServer): void {
+  if (!server.httpServer) return


Since we use httpServer.on('close', ...) to unbind the shortcuts, we can't bind the shortcuts without it. This means middleware mode is off limits.

Shinigami92 · 2021-08-01T10:51:29Z

@aleclarson @HomyeeKing What's the state of this PR?
Seems there are some issues in the tests and it needs a rebase?

@aleclarson Did you take this branch over or just help @HomyeeKing?

HomyeeKing · 2021-08-02T02:30:02Z

@Shinigami92 I will be in this week, kind of busy recently😓

brillout · 2021-08-02T08:30:36Z

@HomyeeKing Actually, in general, why would the user want to restart the dev server? Doesn't HMR handle all code update scenarios?

Maybe an alternative would be to implement your reload shortcut feature as a plugin? Especially if we decide to expose a function to reload the dev server as suggested by @frandiox and @dominikg

HomyeeKing · 2021-08-03T11:41:38Z

@brillout as the PR description said, I submit this PR for convenient debugging,
I don't know how you guys debug, for me , I have to rerun vite dev in my project to watch the newest change with the vite in yarn dev

but this PR seems not solve this problem either, I just found that it just restart the server instance, but the code still not change, such as DEBUG info

so @aleclarson you can assess the trading off to decide this pr to go or not

brillout · 2021-08-03T11:53:47Z

I suggest to fix the root problem instead of adding restart shortcuts then.

AFAICT if HMR works as intended then there is no need for restart shortcuts.

aleclarson · 2021-10-06T18:41:26Z

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

+  },
+  {
+    key: 'f',
+    name: 'force restart',


I think "force restart" is too vague. Should be "reoptimize deps" instead, perhaps?

/cc @antfu @Shinigami92 @patak-js

aleclarson · 2021-10-06T18:41:49Z

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

+    key: 'r',
+    name: 'restart',
+    action(server: ViteDevServer): void {
+      restartServer(server)


Let's remove the restart shortcut for now, since the use case is unclear.

agree, I misunderstand the restartServer function before, I wanted to achieve a way that restart current project with the new Vite bundle output for debugging

aleclarson · 2021-12-08T21:25:51Z

Closing in favor of #6014

Shinigami92 added the p2-nice-to-have Not breaking anything but nice to have (priority) label Apr 28, 2021

aleclarson suggested changes May 28, 2021

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

HomyeeKing force-pushed the feat/restart branch from 902a501 to e315add Compare May 28, 2021 17:49

Shinigami92 marked this pull request as draft May 28, 2021 18:03

Shinigami92 suggested changes May 29, 2021

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

Shinigami92 previously approved these changes May 30, 2021

View reviewed changes

HomyeeKing marked this pull request as ready for review May 30, 2021 10:38

aleclarson reviewed May 30, 2021

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

aleclarson reviewed May 30, 2021

View reviewed changes

HomyeeKing dismissed Shinigami92’s stale review via 64562ac May 31, 2021 09:57

Shinigami92 reviewed May 31, 2021

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

packages/vite/src/node/server/index.ts Outdated Show resolved Hide resolved

antfu reviewed May 31, 2021

View reviewed changes

packages/vite/src/node/server/hmr.ts Outdated Show resolved Hide resolved

aleclarson reviewed May 31, 2021

View reviewed changes

packages/vite/src/node/server/hmr.ts Outdated Show resolved Hide resolved

aleclarson force-pushed the feat/restart branch from 4abbcb8 to e37f622 Compare May 31, 2021 16:14

Shinigami92 reviewed May 31, 2021

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

HomyeeKing requested review from aleclarson, antfu and Shinigami92 June 21, 2021 14:43

dominikg mentioned this pull request Jun 25, 2021

Restart Vite server on svelte config file change sveltejs/vite-plugin-svelte#63

Closed

Shinigami92 added the needs rebase label Jul 4, 2021

HomyeeKing force-pushed the feat/restart branch from e800b57 to 95a49f1 Compare July 4, 2021 10:18

This comment has been minimized.

Sign in to view

HomyeeKing marked this pull request as draft July 26, 2021 06:07

feat: bindShortcuts in middlewareMode

d34cb27

HomyeeKing marked this pull request as ready for review July 26, 2021 06:38

HomyeeKing marked this pull request as draft July 26, 2021 12:13

HomyeeKing added 3 commits July 26, 2021 20:55

fix: disable in middlewareMode and memory leak fix

9d6478e

fix: possible memory leak

100c4e5

chore: remove extra

a7dc987

HomyeeKing marked this pull request as ready for review July 26, 2021 13:41

aleclarson added 2 commits July 26, 2021 12:57

Revert "chore: remove extra"

bf65682

This reverts commit a7dc987.

Revert "fix: possible memory leak"

dd8fc66

This reverts commit 100c4e5.

fix: stop reading stdin when httpServer closes

e58de47

Since we only bind shortcuts in the `server.listen` method, we can safely listen for the httpServer "close" event to unbind them. This avoids a memory leak.

aleclarson reviewed Jul 26, 2021

View reviewed changes

aleclarson suggested changes Oct 6, 2021

View reviewed changes

This was referenced Nov 14, 2021

Automatic Restart After Config Change sveltejs/vite-plugin-svelte#210

Closed

feat(dev): expose restart function on ViteDevServer #5723

Merged

aleclarson mentioned this pull request Dec 8, 2021

feat: add CLI keyboard shortcuts #6014

Closed

4 tasks

aleclarson closed this Dec 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(server): add shortcuts for restart server, open browser, & clear cache #3182

feat(server): add shortcuts for restart server, open browser, & clear cache #3182

HomyeeKing commented Apr 28, 2021 •

edited

Loading

aleclarson commented May 28, 2021

Shinigami92 commented May 28, 2021

HomyeeKing commented May 28, 2021 •

edited

Loading

antfu commented May 29, 2021

aleclarson May 30, 2021

antfu May 30, 2021

Shinigami92 commented Jun 1, 2021

This comment has been minimized.

dominikg commented Jun 25, 2021 •

edited

Loading

patak-dev commented Jul 4, 2021 •

edited

Loading

This comment has been minimized.

HomyeeKing commented Jul 26, 2021 •

edited

Loading

dominikg commented Jul 26, 2021 •

edited

Loading

antfu commented Jul 26, 2021

brillout commented Jul 26, 2021

frandiox commented Jul 26, 2021

aleclarson commented Jul 26, 2021 •

edited

Loading

aleclarson Jul 26, 2021

aleclarson Jul 26, 2021

Shinigami92 commented Aug 1, 2021

HomyeeKing commented Aug 2, 2021

brillout commented Aug 2, 2021

HomyeeKing commented Aug 3, 2021

brillout commented Aug 3, 2021

aleclarson Oct 6, 2021

aleclarson Oct 6, 2021

HomyeeKing Oct 11, 2021

aleclarson commented Dec 8, 2021

feat(server): add shortcuts for restart server, open browser, & clear cache #3182

feat(server): add shortcuts for restart server, open browser, & clear cache #3182

Conversation

HomyeeKing commented Apr 28, 2021 • edited Loading

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

aleclarson commented May 28, 2021

Shinigami92 commented May 28, 2021

HomyeeKing commented May 28, 2021 • edited Loading

antfu commented May 29, 2021

aleclarson May 30, 2021

Choose a reason for hiding this comment

antfu May 30, 2021

Choose a reason for hiding this comment

Shinigami92 commented Jun 1, 2021

This comment has been minimized.

dominikg commented Jun 25, 2021 • edited Loading

patak-dev commented Jul 4, 2021 • edited Loading

This comment has been minimized.

HomyeeKing commented Jul 26, 2021 • edited Loading

dominikg commented Jul 26, 2021 • edited Loading

antfu commented Jul 26, 2021

brillout commented Jul 26, 2021

frandiox commented Jul 26, 2021

aleclarson commented Jul 26, 2021 • edited Loading

aleclarson Jul 26, 2021

Choose a reason for hiding this comment

aleclarson Jul 26, 2021

Choose a reason for hiding this comment

Shinigami92 commented Aug 1, 2021

HomyeeKing commented Aug 2, 2021

brillout commented Aug 2, 2021

HomyeeKing commented Aug 3, 2021

brillout commented Aug 3, 2021

aleclarson Oct 6, 2021

Choose a reason for hiding this comment

aleclarson Oct 6, 2021

Choose a reason for hiding this comment

HomyeeKing Oct 11, 2021

Choose a reason for hiding this comment

aleclarson commented Dec 8, 2021

HomyeeKing commented Apr 28, 2021 •

edited

Loading

HomyeeKing commented May 28, 2021 •

edited

Loading

dominikg commented Jun 25, 2021 •

edited

Loading

patak-dev commented Jul 4, 2021 •

edited

Loading

HomyeeKing commented Jul 26, 2021 •

edited

Loading

dominikg commented Jul 26, 2021 •

edited

Loading

aleclarson commented Jul 26, 2021 •

edited

Loading