Fix --port=0 option to correctly open on random port
#1883
Conversation
🦋 Changeset detectedLatest commit: d744570 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/3532075809/npm-package-wrangler-1883You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1883/npm-package-wrangler-1883Or you can use npx https://prerelease-registry.developers.workers.dev/runs/3532075809/npm-package-wrangler-1883 dev path/to/script.jsAdditional artifacts:npm install https://prerelease-registry.developers.workers.dev/runs/3532075809/npm-package-cloudflare-pages-shared-1883 |
Codecov Report
@@ Coverage Diff @@
## main #1883 +/- ##
==========================================
- Coverage 71.88% 71.88% -0.01%
==========================================
Files 150 150
Lines 9525 9524 -1
Branches 2469 2468 -1
==========================================
- Hits 6847 6846 -1
Misses 2678 2678
|
GregBrimble
force-pushed
the
fix-port-0
branch
from
2144f0f
to
ae624ca
Compare
|
Hey so with cloudflare/miniflare#382 merged, is this good to go? |
|
Just waiting on a Miniflare release 😊 |
|
Also may want to add this to |
|
@mrbbot pls |
GregBrimble
force-pushed
the
fix-port-0
branch
from
ae624ca
to
9249f48
Compare
|
hmm, this actually isn't quite ready. It improves things, but it doesn't update the hotkey things, so pressing |
GregBrimble
force-pushed
the
fix-port-0
branch
2 times, most recently
from
980f3f1
to
8deef81
Compare
| @@ -171,25 +171,39 @@ export function DevImplementation(props: DevProps): JSX.Element { | |||
| ); | |||
| } | |||
|
|
|||
| let ip: string; | |||
There was a problem hiding this comment.
Sheer madness, I know.
There was a problem hiding this comment.
The problem is that useHotkeys() relies on these values for it's b "open a browser" feature. If we store this in proper React state, it gets the value fine, but we then need to call setIp or whatever in the onReady callback. This causes the entire component to re-render, which re-initializes Miniflare, which causes the onReady callback to be called again, etc. etc.
I tried an if (newValue !== newValue) { setValue() }, but still couldn't prevent a render loop.
GregBrimble
force-pushed
the
fix-port-0
branch
from
8deef81
to
d744570
Compare
| port={args.port || configParam.dev.port || (await getLocalPort())} | ||
| ip={args.ip || configParam.dev.ip} | ||
| initialPort={ | ||
| args.port ?? configParam.dev.port ?? (await getLocalPort()) |
There was a problem hiding this comment.
I'm going to come back and refactor getLocalPort and waitForPortToBeAvailable so they're no longer race-y in a subsequent PR.
| abortSignal: abortController.signal, | ||
| }); | ||
| } | ||
| await waitForPortToBeAvailable(initialPort, { |
There was a problem hiding this comment.
if initialPort is 0, we don't have to wait for the port available though - node should just give us an available one?
| @@ -651,6 +651,11 @@ export async function waitForPortToBeAvailable( | |||
| } | |||
|
|
|||
| function checkPort() { | |||
| if (port === 0) { | |||
There was a problem hiding this comment.
Yup. I'll tidy this up in a subsequent PR removing these race-y functions.
Dependent on cloudflare/miniflare#382