React-free hotkeys implementation #6149

RamIdeas · 2024-06-25T23:27:56Z

What this PR solves / how to test

Implements a React-free hotkeys implementation.

You can see both the implementations below – the React-free version with the --x-dev-env flag and the React/ink version with the flag:

I started with a simpler implementation in the first commit but felt a higher-level API would allow for most consistency for other commands wanting to use hotkeys. The instructions printing logic has been moved inside the API and provides each handler with a printInstructions function if the handler wishes to reprint them.

UPDATE:
The instructions box now floats at the bottom (by making Wrangler's Logger and Miniflare's Log classes aware of a possible bottom float string). See image below for what it looks like now (note the worker logs are now above the instructions box):

Author has addressed the following

Tests
- TODO (before merge)
- Included
- Not necessary because:
E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required / Maybe required
- Not required because: hotkeys not currently tested with e2e
Changeset (Changeset guidelines)
- TODO (before merge)
- Included
- Not necessary because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s):
- Not necessary because: experimental

changeset-bot · 2024-06-25T23:28:00Z

🦋 Changeset detected

Latest commit: 1887c45

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
wrangler	Patch
@cloudflare/vitest-pool-workers	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-06-25T23:30:11Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-wrangler-6149

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6149/npm-package-wrangler-6149

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-wrangler-6149 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-create-cloudflare-6149 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-cloudflare-kv-asset-handler-6149

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-miniflare-6149

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-cloudflare-pages-shared-6149

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9763657552/npm-package-cloudflare-vitest-pool-workers-6149

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.62.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240620.0
`workerd`	1.20240701.0	1.20240701.0
`workerd --version`	1.20240701.0	2024-07-01

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

packages/wrangler/src/dev.tsx

packages/miniflare/src/shared/log.ts

packages/wrangler/src/utils/onKeyPress.ts

packages/wrangler/src/dev.tsx

packages/wrangler/src/logger.ts

.changeset/few-days-tap.md

packages/miniflare/src/shared/log.ts

packages/wrangler/src/dev.tsx

packages/wrangler/src/dev/inspect.ts

packages/wrangler/src/dev/use-esbuild.ts

packages/wrangler/src/utils/onKeyPress.ts

petebacondarwin · 2024-07-01T20:35:59Z

packages/wrangler/src/utils/onKeyPress.ts

+
+	if (process.stdin.isTTY) {
+		readline.emitKeypressEvents(stream);
+		process.stdin.setRawMode(true);


Should we capture what the current raw mode is to reuse in the teardown function?

This reminds me I need to test this with the auth/account prompts

Looks like we'll need to setRawMode(true) again after prompts have been used

(my manual testing just now found that prompts work fine but the hotkeys don't work after)

penalosa · 2024-07-02T13:27:00Z

packages/wrangler/src/cli-hotkeys.ts

@@ -8,6 +8,7 @@ import type { Hook } from "./api";
 export default function (
 	options: Array<{
 		keys: string[];
+		disabled?: boolean;


Why do we have a disabled option? Can't we just not pass it in the array?

Felt it was easier to read a disabled flag in an flat-ordered list than the ternary+spread alternative

9b7e52f

packages/wrangler/src/dev.tsx

automatically compute instructions

for mocking

+ ensures no existing logs (from before registerGlobalBottomFloat was called) are cleared

+ use static methods of miniflare Log which are now stateful -- rather than reimplement in Wrangler's Logger not sharing state

RamIdeas requested a review from a team as a code owner June 25, 2024 23:27

cloudflare deleted a comment Jun 25, 2024

RamIdeas force-pushed the startWorker-hotkeys branch from 189870c to 50f1299 Compare June 26, 2024 14:28

workers-devprod added the e2e Run e2e tests on a PR label Jun 26, 2024

RamIdeas force-pushed the startWorker-hotkeys branch 4 times, most recently from 154ce0b to 1079c6d Compare June 27, 2024 11:17

penalosa reviewed Jun 27, 2024

View reviewed changes

packages/wrangler/src/dev.tsx Outdated Show resolved Hide resolved

packages/wrangler/src/dev.tsx Outdated Show resolved Hide resolved

penalosa mentioned this pull request Jun 28, 2024

Chain Reaction #6161

Merged

12 tasks

RamIdeas force-pushed the startWorker-hotkeys branch from ded843c to 8bb83d3 Compare July 1, 2024 14:51

penalosa reviewed Jul 1, 2024

View reviewed changes

packages/wrangler/src/dev.tsx Outdated Show resolved Hide resolved

penalosa reviewed Jul 1, 2024

View reviewed changes

packages/wrangler/src/logger.ts Outdated Show resolved Hide resolved

RamIdeas force-pushed the startWorker-hotkeys branch 2 times, most recently from 558c13a to 8dccb57 Compare July 1, 2024 19:39

petebacondarwin reviewed Jul 1, 2024

View reviewed changes

petebacondarwin approved these changes Jul 2, 2024

View reviewed changes

RamIdeas force-pushed the startWorker-hotkeys branch from 3bc1fd1 to ebafd22 Compare July 2, 2024 11:51

penalosa approved these changes Jul 2, 2024

View reviewed changes

RamIdeas added 8 commits July 2, 2024 15:01

initial implementation

5ec9b89

implement higher-level interface for hotkeys

b9d6ab3

automatically compute instructions

extract onKeyPress to own module

5dc1239

for mocking

add tests

8a2b3da

add changeset

72f00b3

add --x-dev-env flagged run of fixtures/interactive-dev-tests

2a01195

add test for handlers throwing errors

1bcc81e

chore: cleanup teardown

6a84b31

RamIdeas added 23 commits July 2, 2024 15:01

unregisterHotKeys before devEnv.teardown

52495f9

enable modifier key detection in hotkeys api

73f24a0

fix: store previousBottomFloatLineCount in case the line count changes

31bd4c0

+ ensures no existing logs (from before registerGlobalBottomFloat was called) are cleared

fix: usages of console.dir/table -> logger.dir/table

34d284b

+ use static methods of miniflare Log which are now stateful -- rather than reimplement in Wrangler's Logger not sharing state

mark methods with unsafe_

dea4e1a

fixup

fd98482

use setIsTTY inside beforeEach

64ed5be

use readline.moveCursor(stdout,...) instead of stdout.moveCursor(...)

7a47676

abstract knowledge of bototm float from logger implementations

7c11678

fix lints

657ff05

use readline.clearScreenDown

1fd47ed

handle forceLocal (disabled hotkey options)

3c68a51

refactor

c7e32fe

update changeset type to refactor

2758c28

implement logger.console

435ec5d

unregister hotkeys during auth prompt

ff186c6

update AbortError checks

bcbbb68

fix logger.console types

5c5b93b

change getAccountId to requireAuth

22cf654

check isTTY in unregister keypress

01937a0

fix RemoteRuntimeController shutdown log

f1a6764

use isNonInteractiveOrCI util

a88b702

hookify the hotkeys disabled option

9b7e52f

RamIdeas force-pushed the startWorker-hotkeys branch 2 times, most recently from 6db7f95 to 4b4d56a Compare July 2, 2024 15:09

use isInteractive() instead of !isNonIteractiveOrCI()

1887c45

RamIdeas force-pushed the startWorker-hotkeys branch from 4b4d56a to 1887c45 Compare July 2, 2024 15:17

RamIdeas merged commit 35689ea into main Jul 2, 2024
21 checks passed

RamIdeas deleted the startWorker-hotkeys branch July 2, 2024 16:26

workers-devprod mentioned this pull request Jul 2, 2024

Version Packages #6153

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React-free hotkeys implementation #6149

React-free hotkeys implementation #6149

RamIdeas commented Jun 25, 2024 •

edited

Loading

changeset-bot bot commented Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 25, 2024 •

edited

Loading

petebacondarwin Jul 1, 2024

RamIdeas Jul 1, 2024

RamIdeas Jul 1, 2024

RamIdeas Jul 2, 2024

penalosa Jul 2, 2024

RamIdeas Jul 2, 2024

React-free hotkeys implementation #6149

React-free hotkeys implementation #6149

Conversation

RamIdeas commented Jun 25, 2024 • edited Loading

What this PR solves / how to test

Author has addressed the following

changeset-bot bot commented Jun 25, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Jun 25, 2024 • edited Loading

petebacondarwin Jul 1, 2024

Choose a reason for hiding this comment

RamIdeas Jul 1, 2024

Choose a reason for hiding this comment

RamIdeas Jul 1, 2024

Choose a reason for hiding this comment

RamIdeas Jul 2, 2024

Choose a reason for hiding this comment

penalosa Jul 2, 2024

Choose a reason for hiding this comment

RamIdeas Jul 2, 2024

Choose a reason for hiding this comment

RamIdeas commented Jun 25, 2024 •

edited

Loading

changeset-bot bot commented Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 25, 2024 •

edited

Loading