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

Ctrl+C no longer kills processes running in parallel with vite #11434

Closed
7 tasks done
stephenlrandall opened this issue Dec 20, 2022 · 18 comments · Fixed by #11518
Closed
7 tasks done

Ctrl+C no longer kills processes running in parallel with vite #11434

stephenlrandall opened this issue Dec 20, 2022 · 18 comments · Fixed by #11518

Comments

@stephenlrandall
Copy link
Contributor

Describe the bug

When using npm-run-all to run package scripts in parallel with the Vite dev process, a Ctrl+C input only kills the Vite process and a second Ctrl+C is needed to kill the remaining scripts. This is a regression from 3.2.5, where a single Ctrl+C killed all parallel running processes. This may be due to the new shortcuts CLI.

Reproduction

https://github.com/stephenlrandall/vite-cli-ctrl-c-bug

Steps to reproduce

Follow the reproduction README.

System Info

System:
    OS: macOS 13.0.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 21.14 GB / 64.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 17.2.0 - /opt/homebrew/bin/node
    npm: 8.19.3 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 108.0.5359.124
    Safari: 16.1
  npmPackages:
    vite: ^4.0.0 => 4.0.2

Used Package Manager

npm

Logs

No response

Validations

@liangxiwei

This comment was marked as duplicate.

@kinfuy
Copy link
Contributor

kinfuy commented Dec 28, 2022

I guess it's not vite's problem mysticatea/npm-run-all#74

@stephenlrandall
Copy link
Contributor Author

I guess it's not vite's problem mysticatea/npm-run-all#74

It is a Vite problem. This is a new issue in Vite 4 and was not present in Vite 3.2.5. You can see in the reproduction that other scripts when running in parallel can still be killed with a single Ctrl+C.

@stephenlrandall
Copy link
Contributor Author

stephenlrandall commented Dec 28, 2022

@kinfuy I can trace the issue to this segment of bindShortcuts (authored by @ArnaudBarre):

if (process.stdin.isTTY) {
process.stdin.setRawMode(true)
}

About setRawMode:

When in raw mode, input is always available character-by-character, not including modifiers. Additionally, all special processing of characters by the terminal is disabled, including echoing input characters. Ctrl+C will no longer cause a SIGINT when in this mode.

Commenting this line out causes Ctrl+C to work again, but I presume causes other issues with the CLI. Ideally, Vite would be able to do what it needs to without hijacking the input stream.

@stephenlrandall
Copy link
Contributor Author

In the absence of any other solutions, an option to disable the shortcuts CLI (preventing the bindShortcuts function from being run in the first place) would also solve this issue.

@kinfuy
Copy link
Contributor

kinfuy commented Dec 29, 2022

setRawMode It will cause the command line to press ctrl+c and no longer send the termination signal

@kinfuy
Copy link
Contributor

kinfuy commented Dec 29, 2022

// ctrl+c or ctrl+d
if (input === '\x03' || input === '\x04') {

I see there are codes in the main branch to handle this

@kinfuy
Copy link
Contributor

kinfuy commented Dec 29, 2022

I guess this is the problem. In the case of an output stream, process. exit () cannot exit

@ArnaudBarre
Copy link
Member

Maybe we can rethink how the cli should exit in case of ctrl+c. But currently with the new shortcuts, we are able to cleanly exit the CLI and the 0 exit code is not totally wrong. I think this would be kept as is for the q shortcut.

Multiple others options for you:

  • Change your other watch process to be binded with Vite via plugins. This is what I do for tsc watch for example.
  • Starts Vite dev server with the JS API. This doesn't enable shortcuts
  • Use -r for the npm-run-all command to exit when Vite exit with code 0.

@kinfuy
Copy link
Contributor

kinfuy commented Dec 29, 2022

I think that ctrl c represents the forced termination by the user. It is normal for the exit code to be 1. The shortcut key provided by vite means to exit normally. At this time, it is obviously impossible to exit normally, so the exit code is 0. It is reasonable to not respond to this operation.

@kinfuy
Copy link
Contributor

kinfuy commented Dec 29, 2022

If I have to choose one of the three suggestions, I feel it is inappropriate to choose the second and third. First, I have to study it

@stephenlrandall
Copy link
Contributor Author

  • Starts Vite dev server with the JS API. This doesn't enable shortcuts

This should work for my use case; I didn't realize there was already a way to start the Vite dev process without shortcuts. Thanks!

I'll leave the issue open while you guys are still discussing how to handle Ctrl+C input in general.

@kinfuy
Copy link
Contributor

kinfuy commented Jan 4, 2023

Maybe we can rethink how the cli should exit in case of ctrl+c. But currently with the new shortcuts, we are able to cleanly exit the CLI and the 0 exit code is not totally wrong. I think this would be kept as is for the q shortcut.

Multiple others options for you:

  • Change your other watch process to be binded with Vite via plugins. This is what I do for tsc watch for example.
  • Starts Vite dev server with the JS API. This doesn't enable shortcuts
  • Use -r for the npm-run-all command to exit when Vite exit with code 0.

I have a question. How can I know that the JS API started vite

@ArnaudBarre
Copy link
Member

You can use something like this:

    const server = await createServer();
    await server.listen();
    server.printUrls();

More documentation: https://vitejs.dev/guide/api-javascript.html#createserver

@kinfuy
Copy link
Contributor

kinfuy commented Jan 5, 2023

You can use something like this:

    const server = await createServer();
    await server.listen();
    server.printUrls();

More documentation: https://vitejs.dev/guide/api-javascript.html#createserver

It should be my description. What I think is that when judging ctrl c, if we can know that it is called by npm run all or other sub processes, we will give a prompt that ctrl c needs to exit twice

like this:

  if (input === '\x03' || input === '\x04') {
      // TODO
      if(){
        server.config.logger.warn(
          colors.yellow(
            'starts vite dev server with the JS API ctrl c or ctrl d need double',
          ),
        )
      }
      process.emit('SIGTERM')
      return
    }

I think this is necessary

import readline from 'node:readline'
readline.emitKeypressEvents(process.stdin)

@ArnaudBarre
Copy link
Member

If I can detect that Vite is running via npm-run-all, I will skip the shortcuts are this kind cause conflicts with others processes.

@ArnaudBarre
Copy link
Member

I can't find a way to detect if running from npm-run-all or concurrently.
So for me we have three possibilities:

  • Keep it like this (i.e -> ctrl+c -> exit 0) and tell people to use r for npm-run-all or -k for concurrently
  • Add an yet another option
  • Exit one (fix(cli): exit 1 on ctrl+c #11563). The issue is that normal usage become a bit more noisy.

Let's discuss this in the next meeting

@stephenlrandall
Copy link
Contributor Author

Closing with the #11563 merge. Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Feb 15, 2023
futurGH pushed a commit to futurGH/vite that referenced this issue Feb 26, 2023
Co-authored-by: Arnaud Barré <arnaud.barre72@gmail.com>
close vitejs#11434
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants