Skip to content

Commit

Permalink
Fix reporting when using port 0 (#1883)
Browse files Browse the repository at this point in the history
  • Loading branch information
GregBrimble committed Nov 23, 2022
1 parent 12254c3 commit 60d31c0
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 75 deletions.
5 changes: 5 additions & 0 deletions .changeset/yellow-zebras-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: Fix `--port=0` option to report the actually used port.
16 changes: 8 additions & 8 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ describe("wrangler dev", () => {
});
fs.writeFileSync("index.js", `export default {};`);
await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].ip).toEqual("0.0.0.0");
expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("0.0.0.0");
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand All @@ -807,7 +807,7 @@ describe("wrangler dev", () => {
});
fs.writeFileSync("index.js", `export default {};`);
await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].ip).toEqual("1.2.3.4");
expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("1.2.3.4");
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand All @@ -822,7 +822,7 @@ describe("wrangler dev", () => {
});
fs.writeFileSync("index.js", `export default {};`);
await runWrangler("dev --ip=5.6.7.8");
expect((Dev as jest.Mock).mock.calls[0][0].ip).toEqual("5.6.7.8");
expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("5.6.7.8");
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand Down Expand Up @@ -933,7 +933,7 @@ describe("wrangler dev", () => {
});
fs.writeFileSync("index.js", `export default {};`);
await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].port).toEqual(8787);
expect((Dev as jest.Mock).mock.calls[0][0].initialPort).toEqual(8787);
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand All @@ -951,7 +951,7 @@ describe("wrangler dev", () => {
(getPort as jest.Mock).mockResolvedValue(98765);

await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].port).toEqual(8888);
expect((Dev as jest.Mock).mock.calls[0][0].initialPort).toEqual(8888);
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand Down Expand Up @@ -985,7 +985,7 @@ describe("wrangler dev", () => {
(getPort as jest.Mock).mockResolvedValue(98765);

await runWrangler("dev --port=9999");
expect((Dev as jest.Mock).mock.calls[0][0].port).toEqual(9999);
expect((Dev as jest.Mock).mock.calls[0][0].initialPort).toEqual(9999);
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand All @@ -1000,7 +1000,7 @@ describe("wrangler dev", () => {
(getPort as jest.Mock).mockResolvedValue(98765);

await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].port).toEqual(98765);
expect((Dev as jest.Mock).mock.calls[0][0].initialPort).toEqual(98765);
expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
Expand Down Expand Up @@ -1030,7 +1030,7 @@ describe("wrangler dev", () => {
});
fs.writeFileSync("index.js", `export default {};`);
await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].ip).toEqual("0.0.0.0");
expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("0.0.0.0");
expect(std.out).toMatchInlineSnapshot(`
"Your worker has access to the following bindings:
- Durable Objects:
Expand Down
21 changes: 10 additions & 11 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -467,11 +467,13 @@ export async function startDev(args: StartDevOptions) {
accountId={configParam.account_id || getAccountFromCache()?.id}
assetPaths={assetPaths}
assetsConfig={configParam.assets}
port={args.port || configParam.dev.port || (await getLocalPort())}
ip={args.ip || configParam.dev.ip}
initialPort={
args.port ?? configParam.dev.port ?? (await getLocalPort())
}
initialIp={args.ip || configParam.dev.ip}
inspectorPort={
args.inspectorPort ||
configParam.dev.inspector_port ||
args.inspectorPort ??
configParam.dev.inspector_port ??
(await getInspectorPort())
}
isWorkersSite={Boolean(args.site || configParam.site)}
Expand Down Expand Up @@ -584,14 +586,11 @@ export async function startApiDev(args: StartDevOptions) {
assetPaths: assetPaths,
assetsConfig: configParam.assets,
//port can be 0, which means to use a random port
port:
args.port === 0
? args.port
: args.port || configParam.dev.port || (await getLocalPort()),
ip: args.ip || configParam.dev.ip,
initialPort: args.port ?? configParam.dev.port ?? (await getLocalPort()),
initialIp: args.ip || configParam.dev.ip,
inspectorPort:
args["inspector-port"] ||
configParam.dev.inspector_port ||
args["inspector-port"] ??
configParam.dev.inspector_port ??
(await getInspectorPort()),
isWorkersSite: Boolean(args.site || configParam.site),
compatibilityDate: getDevCompatibilityDate(
Expand Down
48 changes: 28 additions & 20 deletions packages/wrangler/src/dev/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ export type DevProps = {
name: string | undefined;
noBundle: boolean;
entry: Entry;
port: number;
ip: string;
initialPort: number;
initialIp: string;
inspectorPort: number;
rules: Config["rules"];
accountId: string | undefined;
Expand Down Expand Up @@ -171,25 +171,42 @@ export function DevImplementation(props: DevProps): JSX.Element {
);
}

// This is a nasty hack to allow `useHotkeys` and its "[b] open a browser" feature to read these values
// without triggering a re-render loop when `onReady()` updates them.
// The initially requested port can be different than what's actually used, if, for example, you request port 0.
let ip: string;
let port: number;

function InteractiveDevSession(props: DevProps) {
const toggles = useHotkeys({
initial: {
local: props.initialMode === "local",
tunnel: false,
},
port: props.port,
ip: props.ip,
inspectorPort: props.inspectorPort,
inspect: props.inspect,
localProtocol: props.localProtocol,
forceLocal: props.forceLocal,
});

ip = props.initialIp;
port = props.initialPort;

useTunnel(toggles.tunnel);

const onReady = (newIp: string, newPort: number) => {
if (newIp !== props.initialIp || newPort !== props.initialPort) {
ip = newIp;
port = newPort;
if (props.onReady) {
props.onReady(newIp, newPort);
}
}
};

return (
<>
<DevSession {...props} local={toggles.local} />
<DevSession {...props} local={toggles.local} onReady={onReady} />
<Box borderStyle="round" paddingLeft={1} paddingRight={1}>
<Text bold={true}>[b]</Text>
<Text> open a browser, </Text>
Expand Down Expand Up @@ -312,8 +329,8 @@ function DevSession(props: DevSessionProps) {
bindings={props.bindings}
workerDefinitions={workerDefinitions}
assetPaths={props.assetPaths}
port={props.port}
ip={props.ip}
initialPort={props.initialPort}
initialIp={props.initialIp}
rules={props.rules}
inspectorPort={props.inspectorPort}
localPersistencePath={props.localPersistencePath}
Expand All @@ -339,8 +356,8 @@ function DevSession(props: DevSessionProps) {
bindings={props.bindings}
assetPaths={props.assetPaths}
isWorkersSite={props.isWorkersSite}
port={props.port}
ip={props.ip}
port={props.initialPort}
ip={props.initialIp}
localProtocol={props.localProtocol}
inspectorPort={props.inspectorPort}
// TODO: @threepointone #1167
Expand Down Expand Up @@ -503,25 +520,16 @@ type useHotkeysInitialState = {
};
function useHotkeys(props: {
initial: useHotkeysInitialState;
port: number;
ip: string;
inspectorPort: number;
inspect: boolean;
localProtocol: "http" | "https";
forceLocal: boolean | undefined;
}) {
const {
initial,
port,
ip,
inspectorPort,
inspect,
localProtocol,
forceLocal,
} = props;
const { initial, inspectorPort, inspect, localProtocol, forceLocal } = props;
// UGH, we should put port in context instead
const [toggles, setToggles] = useState(initial);
const { exit } = useApp();

useInput(
async (
input,
Expand Down
32 changes: 18 additions & 14 deletions packages/wrangler/src/dev/local.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ export interface LocalProps {
bindings: CfWorkerInit["bindings"];
workerDefinitions: WorkerRegistry | undefined;
assetPaths: AssetPaths | undefined;
port: number;
ip: string;
initialPort: number;
initialIp: string;
rules: Config["rules"];
inspectorPort: number;
localPersistencePath: string | null;
Expand Down Expand Up @@ -109,12 +109,12 @@ function useLocalWorker({
bindings,
workerDefinitions,
assetPaths,
port,
initialPort,
inspectorPort,
rules,
localPersistencePath,
liveReload,
ip,
initialIp,
crons,
queueConsumers,
localProtocol,
Expand Down Expand Up @@ -160,7 +160,7 @@ function useLocalWorker({
if (!bundle || !format) return;

// port for the worker
await waitForPortToBeAvailable(port, {
await waitForPortToBeAvailable(initialPort, {
retryPeriod: 200,
timeout: 2000,
abortSignal: abortController.signal,
Expand Down Expand Up @@ -199,10 +199,10 @@ function useLocalWorker({

const { forkOptions, miniflareCLIPath, options } = setupMiniflareOptions({
workerName,
port,
port: initialPort,
scriptPath,
localProtocol,
ip,
ip: initialIp,
format,
rules,
compatibilityDate,
Expand Down Expand Up @@ -295,7 +295,11 @@ function useLocalWorker({
return;
}

const nodeOptions = setupNodeOptions({ inspect, ip, inspectorPort });
const nodeOptions = setupNodeOptions({
inspect,
ip: initialIp,
inspectorPort,
});
logger.log("⎔ Starting a local server...");

const child = (local.current = fork(miniflareCLIPath, forkOptions, {
Expand All @@ -316,21 +320,21 @@ function useLocalWorker({
await registerWorker(workerName, {
protocol: localProtocol,
mode: "local",
port,
host: ip,
port: message.port,
host: initialIp,
durableObjects: internalDurableObjects.map((binding) => ({
name: binding.name,
className: binding.class_name,
})),
...(message.durableObjectsPort
? {
durableObjectsHost: ip,
durableObjectsHost: initialIp,
durableObjectsPort: message.durableObjectsPort,
}
: {}),
});
}
onReady?.(ip, message.mfPort);
onReady?.(initialIp, message.port);
}
});

Expand Down Expand Up @@ -399,9 +403,9 @@ function useLocalWorker({
bundle,
workerName,
format,
port,
initialPort,
inspectorPort,
ip,
initialIp,
queueConsumers,
bindings.queues,
bindings.durable_objects,
Expand Down

0 comments on commit 60d31c0

Please sign in to comment.