Skip to content

Commit 7d50da6

Browse files
committed
fix: address CodeRabbit review findings
- cli(next): gate DistService instantiation behind distZip !== false, matching the Vite plugin path so sync is correctly disabled when distZip: false - cli(next): destroy unhandled upgrade sockets instead of leaving them open - pp-ws-server: implement send(payload) overload — non-string payloads were silently dropped; now broadcast as raw JSON like the string overload - hot-context: cap outbox at 50 entries (drop oldest) to prevent unbounded memory growth during prolonged disconnects - test(load-pp-data): await server.close() callbacks to prevent flaky parallel tests - test(pp-ws-server): yield an event-loop tick before asserting broadcast isolation to eliminate the race condition in the per-client isolation test
1 parent 0363f63 commit 7d50da6

5 files changed

Lines changed: 35 additions & 12 deletions

File tree

src/cli.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,7 @@ cli
608608
proxyCacheTTL = 600000,
609609
personalAccessToken = process.env.MI_ACCESS_TOKEN,
610610
miHudLess = false,
611+
distZip = true,
611612
} = ppDevConfig;
612613

613614
const appId: number =
@@ -1044,14 +1045,17 @@ cli
10441045
// Fall back to defaults if the project package.json is unreadable.
10451046
}
10461047

1047-
const distService = new DistService(templateName ?? basename(projectRoot), {
1048-
nextBuild: {
1049-
projectRoot,
1050-
distDir: nextExportDir,
1051-
packageVersion: nextPackageVersion,
1052-
packageRepositoryUrl: nextPackageRepositoryUrl,
1053-
},
1054-
});
1048+
const distService =
1049+
distZip !== false
1050+
? new DistService(templateName ?? basename(projectRoot), {
1051+
nextBuild: {
1052+
projectRoot,
1053+
distDir: nextExportDir,
1054+
packageVersion: nextPackageVersion,
1055+
packageRepositoryUrl: nextPackageRepositoryUrl,
1056+
},
1057+
})
1058+
: undefined;
10551059

10561060
// Minimal ViteDevServer shape consumed by ClientService (`ws` + the v7 flag).
10571061
const clientServiceServer = {
@@ -1073,6 +1077,8 @@ cli
10731077

10741078
if (nextUpgradeHandler) {
10751079
nextUpgradeHandler(req, socket, head);
1080+
} else {
1081+
socket.destroy();
10761082
}
10771083
});
10781084

src/client/hot-context.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ type CustomMessage = { type: 'custom'; event: string; data?: unknown };
2121

2222
type Handler = (payload: any) => void;
2323

24+
const MAX_OUTBOX_SIZE = 50;
25+
2426
function resolveWebSocketUrl(): string {
2527
const { protocol, host } = window.location;
2628
const wsProtocol = protocol === 'https:' ? 'wss:' : 'ws:';
@@ -141,6 +143,9 @@ export function createPPDevHotContext(): ViteHotContext {
141143
if (socket && socket.readyState === WebSocket.OPEN) {
142144
socket.send(payload);
143145
} else {
146+
if (outbox.length >= MAX_OUTBOX_SIZE) {
147+
outbox.shift();
148+
}
144149
outbox.push(payload);
145150
}
146151
},

src/lib/pp-ws-server.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ export class PPDevHotServer {
7171
for (const socket of this.clients.keys()) {
7272
this.sendToSocket(socket, arg1, arg2);
7373
}
74+
} else {
75+
// send(payload: unknown) overload — broadcast the pre-built payload as-is.
76+
const data = JSON.stringify(arg1);
77+
for (const socket of this.clients.keys()) {
78+
if (socket.readyState === WebSocket.OPEN) {
79+
socket.send(data);
80+
}
81+
}
7482
}
7583
},
7684
};

tests/integration/middleware/load-pp-data.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('initLoadPPData — load on deep-linked sub-path navigation', () => {
7575

7676
expect(getPageVariables).toHaveBeenCalledTimes(1);
7777
} finally {
78-
server.close();
78+
await new Promise<void>((resolve) => server.close(() => resolve()));
7979
}
8080
});
8181

@@ -87,7 +87,7 @@ describe('initLoadPPData — load on deep-linked sub-path navigation', () => {
8787

8888
expect(getPageVariables).toHaveBeenCalledTimes(1);
8989
} finally {
90-
server.close();
90+
await new Promise<void>((resolve) => server.close(() => resolve()));
9191
}
9292
});
9393

@@ -99,7 +99,7 @@ describe('initLoadPPData — load on deep-linked sub-path navigation', () => {
9999

100100
expect(getPageVariables).not.toHaveBeenCalled();
101101
} finally {
102-
server.close();
102+
await new Promise<void>((resolve) => server.close(() => resolve()));
103103
}
104104
});
105105

@@ -111,7 +111,7 @@ describe('initLoadPPData — load on deep-linked sub-path navigation', () => {
111111

112112
expect(getPageVariables).not.toHaveBeenCalled();
113113
} finally {
114-
server.close();
114+
await new Promise<void>((resolve) => server.close(() => resolve()));
115115
}
116116
});
117117
});

tests/integration/middleware/pp-ws-server.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ describe('PPDevHotServer transport', () => {
9696

9797
const message = await nextMessage(sender);
9898

99+
// Yield a full event-loop tick so any unintended broadcast would arrive at
100+
// `other` before we assert isolation.
101+
await new Promise<void>((resolve) => setImmediate(resolve));
102+
99103
expect(message.event).toBe('info-data:response');
100104
expect(otherReceived).toBe(false);
101105
} finally {

0 commit comments

Comments
 (0)