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

refactor(shortcuts)!: tweak shortcuts api #14749

Merged
merged 1 commit into from
Oct 26, 2023
Merged

refactor(shortcuts)!: tweak shortcuts api #14749

merged 1 commit into from
Oct 26, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Oct 24, 2023

Description

Tighten up the shortcuts implementation a bit before Vite 5

  • Made customShortcuts only an array of shortcuts. Doesn't support null | undefined array elements.
  • Don't print duplicate keys for h help shortcut. E.g. if user defines the r shortcut to do something else (and not restart the server), we should only print it once.
  • Don't run user-defined h shortcut as we always handle it.

Additional context

One thing I wonder is if we should allow users to override the h shortcut, but that makes the code a bit ugly and could also be tackled non-breakingly in the future.

Slightly related discussion: #8746 (comment)


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@stackblitz
Copy link

stackblitz bot commented Oct 24, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Member

@ArnaudBarre ArnaudBarre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I like the simplicity of passing null & undefined. This is something that rollup and Vite allows (with also false and promise and list)

@bluwy
Copy link
Member Author

bluwy commented Oct 25, 2023

I think for server.bindCLIShortcuts specifically, its API tend to be imperative so we can be a little stricter here. The user can do .filter(Boolean) if needed. Vite configs are a little different where it's more declarative and users want a quick way to short-circuit certain plugins.

@patak-dev patak-dev merged commit 0ae2e1d into main Oct 26, 2023
10 checks passed
@patak-dev patak-dev deleted the shortcut-refactor branch October 26, 2023 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants