Core: Harden websocket connection#33860
Merged
valentinpalkovic merged 5 commits intov9from Feb 18, 2026
Merged
Conversation
valentinpalkovic
added
bug
ci:daily
Contributor
📝 WalkthroughWalkthroughGenerates a per-run wsToken in the core preset, exposes it to framework globals in development, appends it to client channel URLs, and enforces token validation on server WebSocket upgrades using a timing-safe comparison; also adjusts CORS headers and updates tests and changelog. Changes
Sequence DiagramsequenceDiagram
participant Preset as Core Preset
participant Manager as Framework Manager
participant Client as Browser Client
participant Server as Storybook Server
participant WS as WebSocket Upgrade
Preset->>Preset: Generate wsToken (randomUUID)
Preset->>Manager: Emit CoreConfig with channelOptions.wsToken
Manager->>Manager: Store wsToken in globals.CHANNEL_OPTIONS
Client->>Manager: Read globals.CHANNEL_OPTIONS
Client->>Server: Request /storybook-server-channel?token=wsToken
Server->>WS: Receive upgrade for /storybook-server-channel
WS->>WS: Extract token from URL query
WS->>WS: Validate token (isValidToken)
alt Token Valid
WS->>Server: Proceed with upgrade → establish channel
Server->>Client: WebSocket connection established
else Token Invalid
WS->>Client: Respond 403 Forbidden and close socket
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
code/core/src/channels/index.ts (1)
38-39: Consider handling undefined token more gracefully.If
wsTokenis undefined, the URL becomes?token=undefined, which will be rejected by the server. While this fails safely, it could make debugging harder. Consider either:
- Skipping the WebSocket transport entirely if no token is available
- Logging a warning when token is missing
♻️ Option: Skip transport if token unavailable
if (CONFIG_TYPE === 'DEVELOPMENT') { const protocol = window.location.protocol === 'http:' ? 'ws' : 'wss'; const { hostname, port } = window.location; const { wsToken } = CHANNEL_OPTIONS || {}; - const channelUrl = `${protocol}://${hostname}:${port}/storybook-server-channel?token=${wsToken}`; - - transports.push(new WebsocketTransport({ url: channelUrl, onError: () => {}, page })); + if (wsToken) { + const channelUrl = `${protocol}://${hostname}:${port}/storybook-server-channel?token=${wsToken}`; + transports.push(new WebsocketTransport({ url: channelUrl, onError: () => {}, page })); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/channels/index.ts` around lines 38 - 39, CHANNEL_OPTIONS.wsToken may be undefined which yields a URL with "?token=undefined"; update the logic around CHANNEL_OPTIONS / wsToken in the module that builds channelUrl so you handle a missing token: if wsToken is falsy, either (A) skip creating the WebSocket transport entirely (return/omit the transport where channelUrl is used) or (B) emit a clear warning via the existing logger (e.g., processLogger.warn or the module's logger) and build the URL without the token query param; modify the code that computes channelUrl (references: CHANNEL_OPTIONS, wsToken, channelUrl, protocol/hostname/port) to branch on wsToken and avoid embedding the literal "undefined" token.code/core/src/core-server/presets/common-preset.ts (1)
255-257: Clarify the purpose of this passthrough function.The
channelTokenfunction appears to be a no-op passthrough that simply returns its input unchanged. If this is intended as a preset hook for future extensibility, consider adding a JSDoc comment explaining its purpose.📝 Suggested documentation
+/** + * Preset hook for channel token configuration. + * Currently a passthrough, but can be extended by other presets. + */ export const channelToken = async (value: string | undefined) => { return value; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/presets/common-preset.ts` around lines 255 - 257, The function channelToken is a no-op passthrough returning its input; add a concise JSDoc above channelToken explaining its intent as a preset hook/passthrough for potential future token processing or validation, document the parameter (value: string | undefined) and return type, and note any semantic expectations (e.g., may be overridden or extended by presets) so readers understand why it exists despite being a simple return.code/core/src/core-server/utils/__tests__/server-channel.test.ts (2)
95-115: Redundant spy creation onsocket.destroy.Line 99 assigns
vi.fn()tosocket.destroy, which already provides a mock function that can be asserted withtoHaveBeenCalled(). The subsequentvi.spyOn(socket, 'destroy')on line 100 is redundant.♻️ Suggested simplification
it('rejects connections with invalid token', () => { const server = new EventEmitter() as any as Server; const socket = new EventEmitter() as any; socket.write = vi.fn(); socket.destroy = vi.fn(); - const destroySpy = vi.spyOn(socket, 'destroy'); new ServerChannelTransport(server, mockToken); // Simulate upgrade request with wrong token const request = { url: '/storybook-server-channel?token=wrong-token', } as any; const head = Buffer.from(''); server.listeners('upgrade')[0](request, socket, head); expect(socket.write).toHaveBeenCalledWith( 'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n' ); - expect(destroySpy).toHaveBeenCalled(); + expect(socket.destroy).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts` around lines 95 - 115, The test creates a redundant spy: remove the vi.spyOn call for socket.destroy and rely on the existing mock assigned via socket.destroy = vi.fn(); update assertions to check socket.destroy directly (e.g., expect(socket.destroy).toHaveBeenCalled()) instead of using destroySpy; the change affects the test around ServerChannelTransport instantiation and the socket.destroy mock in server-channel.test.ts where destroySpy is currently created.
117-141: Same redundant spy pattern as above.The
destroySpyon line 122 is unnecessary sincesocket.destroyis already avi.fn()mock.♻️ Suggested simplification
it('accepts connections with valid token', () => { const server = new EventEmitter() as any as Server; const socket = new EventEmitter() as any; socket.write = vi.fn(); socket.destroy = vi.fn(); - const destroySpy = vi.spyOn(socket, 'destroy'); const handleUpgradeSpy = vi.fn(); const transport = new ServerChannelTransport(server, mockToken); // Mock handleUpgrade to track if it's called // `@ts-expect-error` (accessing private property) transport.socket.handleUpgrade = handleUpgradeSpy; // Simulate upgrade request with correct token const request = { url: `/storybook-server-channel?token=${mockToken}`, } as any; const head = Buffer.from(''); server.listeners('upgrade')[0](request, socket, head); expect(socket.write).not.toHaveBeenCalled(); - expect(destroySpy).not.toHaveBeenCalled(); + expect(socket.destroy).not.toHaveBeenCalled(); expect(handleUpgradeSpy).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts` around lines 117 - 141, The test creates a redundant spy "destroySpy" for socket.destroy even though socket.destroy is already mocked with vi.fn(); remove the extra vi.spyOn(socket, 'destroy') and references to destroySpy in the 'accepts connections with valid token' test; keep the existing mock assignment socket.destroy = vi.fn(), and assert against socket.destroy directly (or remove the duplicate expectation variable), while leaving ServerChannelTransport, socket.write mock, handleUpgradeSpy, and the upgrade invocation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/core/src/channels/index.ts`:
- Around line 38-39: CHANNEL_OPTIONS.wsToken may be undefined which yields a URL
with "?token=undefined"; update the logic around CHANNEL_OPTIONS / wsToken in
the module that builds channelUrl so you handle a missing token: if wsToken is
falsy, either (A) skip creating the WebSocket transport entirely (return/omit
the transport where channelUrl is used) or (B) emit a clear warning via the
existing logger (e.g., processLogger.warn or the module's logger) and build the
URL without the token query param; modify the code that computes channelUrl
(references: CHANNEL_OPTIONS, wsToken, channelUrl, protocol/hostname/port) to
branch on wsToken and avoid embedding the literal "undefined" token.
In `@code/core/src/core-server/presets/common-preset.ts`:
- Around line 255-257: The function channelToken is a no-op passthrough
returning its input; add a concise JSDoc above channelToken explaining its
intent as a preset hook/passthrough for potential future token processing or
validation, document the parameter (value: string | undefined) and return type,
and note any semantic expectations (e.g., may be overridden or extended by
presets) so readers understand why it exists despite being a simple return.
In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts`:
- Around line 95-115: The test creates a redundant spy: remove the vi.spyOn call
for socket.destroy and rely on the existing mock assigned via socket.destroy =
vi.fn(); update assertions to check socket.destroy directly (e.g.,
expect(socket.destroy).toHaveBeenCalled()) instead of using destroySpy; the
change affects the test around ServerChannelTransport instantiation and the
socket.destroy mock in server-channel.test.ts where destroySpy is currently
created.
- Around line 117-141: The test creates a redundant spy "destroySpy" for
socket.destroy even though socket.destroy is already mocked with vi.fn(); remove
the extra vi.spyOn(socket, 'destroy') and references to destroySpy in the
'accepts connections with valid token' test; keep the existing mock assignment
socket.destroy = vi.fn(), and assert against socket.destroy directly (or remove
the duplicate expectation variable), while leaving ServerChannelTransport,
socket.write mock, handleUpgradeSpy, and the upgrade invocation unchanged.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 515 KB | 🚨 +515 KB 🚨 |
| Dependency size | 0 B | 2.98 MB | 🚨 +2.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-docs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 12 | 🚨 +12 🚨 |
| Self size | 0 B | 2.43 MB | 🚨 +2.43 MB 🚨 |
| Dependency size | 0 B | 9.74 MB | 🚨 +9.74 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-links
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 20 KB | 🚨 +20 KB 🚨 |
| Dependency size | 0 B | 5 KB | 🚨 +5 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-onboarding
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 228 KB | 🚨 +228 KB 🚨 |
| Dependency size | 0 B | 646 B | 🚨 +646 B 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook-addon-pseudo-states
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 47 KB | 🚨 +47 KB 🚨 |
| Dependency size | 0 B | 665 B | 🚨 +665 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-themes
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 21 KB | 🚨 +21 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 6 | 🚨 +6 🚨 |
| Self size | 0 B | 556 KB | 🚨 +556 KB 🚨 |
| Dependency size | 0 B | 1.53 MB | 🚨 +1.53 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 5 | 🚨 +5 🚨 |
| Self size | 0 B | 450 KB | 🚨 +450 KB 🚨 |
| Dependency size | 0 B | 834 KB | 🚨 +834 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 200 | 🚨 +200 🚨 |
| Self size | 0 B | 81 KB | 🚨 +81 KB 🚨 |
| Dependency size | 0 B | 32.28 MB | 🚨 +32.28 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 50 | 🚨 +50 🚨 |
| Self size | 0 B | 36.43 MB | 🚨 +36.43 MB 🚨 |
| Dependency size | 0 B | 16.65 MB | 🚨 +16.65 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 201 | 🚨 +201 🚨 |
| Self size | 0 B | 192 KB | 🚨 +192 KB 🚨 |
| Dependency size | 0 B | 30.57 MB | 🚨 +30.57 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 205 | 🚨 +205 🚨 |
| Self size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Dependency size | 0 B | 29.00 MB | 🚨 +29.00 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 8 | 🚨 +8 🚨 |
| Self size | 0 B | 24 KB | 🚨 +24 KB 🚨 |
| Dependency size | 0 B | 1.33 MB | 🚨 +1.33 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 543 | 🚨 +543 🚨 |
| Self size | 0 B | 881 KB | 🚨 +881 KB 🚨 |
| Dependency size | 0 B | 60.37 MB | 🚨 +60.37 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 130 | 🚨 +130 🚨 |
| Self size | 0 B | 3.12 MB | 🚨 +3.12 MB 🚨 |
| Dependency size | 0 B | 22.94 MB | 🚨 +22.94 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 8 | 🚨 +8 🚨 |
| Self size | 0 B | 14 KB | 🚨 +14 KB 🚨 |
| Dependency size | 0 B | 1.31 MB | 🚨 +1.31 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 163 | 🚨 +163 🚨 |
| Self size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Dependency size | 0 B | 24.29 MB | 🚨 +24.29 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 120 | 🚨 +120 🚨 |
| Self size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Dependency size | 0 B | 20.73 MB | 🚨 +20.73 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 287 | 🚨 +287 🚨 |
| Self size | 0 B | 25 KB | 🚨 +25 KB 🚨 |
| Dependency size | 0 B | 44.73 MB | 🚨 +44.73 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 212 | 🚨 +212 🚨 |
| Self size | 0 B | 17 KB | 🚨 +17 KB 🚨 |
| Dependency size | 0 B | 33.55 MB | 🚨 +33.55 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 14 | 🚨 +14 🚨 |
| Self size | 0 B | 54 KB | 🚨 +54 KB 🚨 |
| Dependency size | 0 B | 26.56 MB | 🚨 +26.56 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 15 | 🚨 +15 🚨 |
| Self size | 0 B | 51 KB | 🚨 +51 KB 🚨 |
| Dependency size | 0 B | 26.61 MB | 🚨 +26.61 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 105 | 🚨 +105 🚨 |
| Self size | 0 B | 34 KB | 🚨 +34 KB 🚨 |
| Dependency size | 0 B | 43.70 MB | 🚨 +43.70 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 9 | 🚨 +9 🚨 |
| Self size | 0 B | 20 KB | 🚨 +20 KB 🚨 |
| Dependency size | 0 B | 1.36 MB | 🚨 +1.36 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
sb
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 51 | 🚨 +51 🚨 |
| Self size | 0 B | 1 KB | 🚨 +1 KB 🚨 |
| Dependency size | 0 B | 53.08 MB | 🚨 +53.08 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 220 | 🚨 +220 🚨 |
| Self size | 0 B | 598 KB | 🚨 +598 KB 🚨 |
| Dependency size | 0 B | 98.55 MB | 🚨 +98.55 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 189 | 🚨 +189 🚨 |
| Self size | 0 B | 31 KB | 🚨 +31 KB 🚨 |
| Dependency size | 0 B | 82.38 MB | 🚨 +82.38 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/core-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 12.76 MB | 🚨 +12.76 MB 🚨 |
| Dependency size | 0 B | 99 KB | 🚨 +99 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/csf-plugin
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 11 KB | 🚨 +11 KB 🚨 |
| Dependency size | 0 B | 796 KB | 🚨 +796 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
eslint-plugin-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 333 KB | 🚨 +333 KB 🚨 |
| Dependency size | 0 B | 3.36 MB | 🚨 +3.36 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-dom-shim
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 10 KB | 🚨 +10 KB 🚨 |
| Dependency size | 0 B | 774 B | 🚨 +774 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-create-react-app
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 68 | 🚨 +68 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 5.99 MB | 🚨 +5.99 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 177 | 🚨 +177 🚨 |
| Self size | 0 B | 24 KB | 🚨 +24 KB 🚨 |
| Dependency size | 0 B | 31.46 MB | 🚨 +31.46 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-server-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 10 | 🚨 +10 🚨 |
| Self size | 0 B | 10 KB | 🚨 +10 KB 🚨 |
| Dependency size | 0 B | 1.20 MB | 🚨 +1.20 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 38 KB | 🚨 +38 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 23 KB | 🚨 +23 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 1.66 MB | 🚨 +1.66 MB 🚨 |
| Dependency size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 13 KB | 🚨 +13 KB 🚨 |
| Dependency size | 0 B | 716 KB | 🚨 +716 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 64 KB | 🚨 +64 KB 🚨 |
| Dependency size | 0 B | 230 KB | 🚨 +230 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 83 KB | 🚨 +83 KB 🚨 |
| Dependency size | 0 B | 211 KB | 🚨 +211 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 57 KB | 🚨 +57 KB 🚨 |
| Dependency size | 0 B | 47 KB | 🚨 +47 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts`:
- Line 91: Remove the leftover debug console.log in the test: delete the
statement that prints handler.mock.calls in server-channel.test.ts (the
console.log(handler.mock.calls); line) so the test contains only assertions and
no direct console output; ensure any needed inspection of handler mock is done
via Jest assertions on handler (e.g., handler.mock.calls or
handler.mock.calls.length) rather than logging.
| const input = { a() {}, b: class {}, c: true, d: 3 }; | ||
| socket.emit('message', stringify(input)); | ||
|
|
||
| console.log(handler.mock.calls); |
Contributor
There was a problem hiding this comment.
Remove debug console.log statement.
This appears to be a leftover debug artifact. As per coding guidelines, console.log should not be used directly in code.
Proposed fix
- console.log(handler.mock.calls);
-
expect(handler.mock.calls[0][0].a).toBeUndefined();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log(handler.mock.calls); | |
| expect(handler.mock.calls[0][0].a).toBeUndefined(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts` at line 91,
Remove the leftover debug console.log in the test: delete the statement that
prints handler.mock.calls in server-channel.test.ts (the
console.log(handler.mock.calls); line) so the test contains only assertions and
no direct console output; ensure any needed inspection of handler mock is done
via Jest assertions on handler (e.g., handler.mock.calls or
handler.mock.calls.length) rather than logging.
github-project-automation
bot
moved this from In Progress
to Done
in Core Team Projects
This was referenced Feb 25, 2026
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes N/A
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-33860-sha-7316283e. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-33860-sha-7316283e sandboxor in an existing project withnpx storybook@0.0.0-pr-33860-sha-7316283e upgrade.More information
0.0.0-pr-33860-sha-7316283ehotfix/v9.1.197316283e1771338267)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=33860Summary by CodeRabbit
New Features
Improvements
Tests
Chores