Skip to content

refactor(shortcuts)!: tweak shortcuts api#14749

Merged
patak-cat merged 1 commit intomainfrom
shortcut-refactor
Oct 26, 2023
Merged

refactor(shortcuts)!: tweak shortcuts api#14749
patak-cat merged 1 commit intomainfrom
shortcut-refactor

Conversation

@bluwy
Copy link
Copy Markdown
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

@bolt-new-by-stackblitz
Copy link
Copy Markdown

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

Copy link
Copy Markdown
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
Copy Markdown
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-cat patak-cat merged commit 0ae2e1d into main Oct 26, 2023
@patak-cat patak-cat 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