fix: eliminate middleware bundle leak via lazy dynamic import#85
Conversation
`authkitMiddleware()` statically imported `authkit-loader.ts`, which pulled `@workos/authkit-session` → `@workos-inc/node` → `eventemitter3` into the client module graph during Vite dev. Unlike `createServerFn` handlers (which the TanStack compiler rewrites to `createClientRpc` stubs), `createMiddleware().server()` callbacks get no such rewrite. Apply the same lazy-body pattern from #75: middleware.ts is now a thin shell that `await import()`s middleware-body.ts inside the `.server()` callback. The dynamic import is dead-code-eliminated from the client graph. Also: - Add middleware.ts to oxlint no-restricted-imports override - Add eventemitter3 to bundle-leak fingerprints - Split tests: shell delegation in middleware.spec.ts, logic in middleware-body.spec.ts Resolves #82.
Greptile SummaryThis PR fixes a Vite dev-mode
Confidence Score: 5/5Safe to merge — the lazy dynamic import correctly severs the server-only import chain from the Vite client graph, and all existing tests pass with the refactored structure. The change is a straightforward extraction of middleware logic into a lazily-loaded module. The getSetCookie() improvement over the old headers.get('Set-Cookie') is a correctness fix for multi-cookie responses. The previously flagged mock shape issue in the session-refresh test has been addressed in the new middleware-body.spec.ts. No regressions or new defects are introduced. No files require special attention. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const mockResponse = new Response('OK', { status: 200 }); | ||
|
|
||
| const args = { | ||
| request: mockRequest, | ||
| next: vi.fn(async () => ({ response: mockResponse })), | ||
| }; | ||
|
|
||
| await middlewareBody(args); | ||
|
|
||
| expect(mockAuthkit.saveSession).toHaveBeenCalledWith(undefined, refreshedData); | ||
| }); | ||
|
|
||
| it('provides correct context shape to downstream handlers', async () => { | ||
| const mockAuth = { user: { id: 'user_123' }, sessionId: 'session_123' }; | ||
| mockAuthkit.withAuth.mockResolvedValue({ | ||
| auth: mockAuth, | ||
| refreshedSessionData: null, | ||
| }); | ||
|
|
There was a problem hiding this comment.
saveSession mock shape doesn't match what the production code reads
saveSession is mocked to resolve with { headers: { 'Set-Cookie': '...' } }, but middlewareBody destructures { response: sessionResponse } from that return value (line 34 of middleware-body.ts). Since response is absent from the mock, sessionResponse is always undefined, sessionResponse?.headers.get('Set-Cookie') returns undefined, and the cookie is never appended — so this test never actually verifies that the refreshed-session Set-Cookie reaches the final response. The mock should resolve with { response: new Response('', { headers: { 'Set-Cookie': 'wos-session=new_value; Path=/' } }) } to exercise that path.
There was a problem hiding this comment.
Yep. The mock here should be
mockAuthkit.saveSession.mockResolvedValue({
response: new Response('', {
headers: { 'Set-Cookie': 'wos-session=new_value; Path=/' },
}),
});and then later
const result = await middlewareBody(args);
expect(result.response.headers.get('Set-Cookie')).toContain('wos-session=new_value');There was a problem hiding this comment.
Fixed in 3b923a1 — corrected the mock to return { response: new Response(...) } and added an assertion that Set-Cookie reaches the final response.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
| const mockResponse = new Response('OK', { status: 200 }); | ||
|
|
||
| const args = { | ||
| request: mockRequest, | ||
| next: vi.fn(async () => ({ response: mockResponse })), | ||
| }; | ||
|
|
||
| await middlewareBody(args); | ||
|
|
||
| expect(mockAuthkit.saveSession).toHaveBeenCalledWith(undefined, refreshedData); | ||
| }); | ||
|
|
||
| it('provides correct context shape to downstream handlers', async () => { | ||
| const mockAuth = { user: { id: 'user_123' }, sessionId: 'session_123' }; | ||
| mockAuthkit.withAuth.mockResolvedValue({ | ||
| auth: mockAuth, | ||
| refreshedSessionData: null, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Yep. The mock here should be
mockAuthkit.saveSession.mockResolvedValue({
response: new Response('', {
headers: { 'Set-Cookie': 'wos-session=new_value; Path=/' },
}),
});and then later
const result = await middlewareBody(args);
expect(result.response.headers.get('Set-Cookie')).toContain('wos-session=new_value');| @@ -0,0 +1,61 @@ | |||
| import { getAuthkit, validateConfig, getConfig } from './authkit-loader.js'; | |||
| import type { AuthKitMiddlewareOptions } from './middleware.js'; | |||
There was a problem hiding this comment.
AuthKitMiddlewareOptions should be pulled out into a separate file.
middleware-body.ts imports AuthKitMiddlewareOptions from ./middleware.js, while middleware.ts dynamically imports middleware-body.ts on its L38. It's a fragile setup that could re-leak this issue in the future.
There was a problem hiding this comment.
Moved AuthKitMiddlewareOptions to src/server/types.ts. middleware.ts re-exports it for public API compatibility, middleware-body.ts imports directly from types — no bidirectional dependency.
| if (refreshedSessionData) { | ||
| const { response: sessionResponse } = await authkit.saveSession(undefined, refreshedSessionData); | ||
| const setCookieHeader = sessionResponse?.headers.get('Set-Cookie'); | ||
| if (setCookieHeader) { |
There was a problem hiding this comment.
Set-Cookie acts a little differently than regular headers. Headers.get() uses header values can be safely comma-joined when there are multiple entries; but Set-Cookie values can legitimately contain commas.
The spec-defined accessor for this is getSetCookie(), which takes this quirk into consideration and returns each cookie as its own array entry. This code should change to:
| if (setCookieHeader) { | |
| for (const cookie of sessionResponse?.headers.getSetCookie() ?? []) { | |
| pendingHeaders.append('Set-Cookie', cookie); | |
| } |
Appending each cookie individually preserves them as separate Set-Cookie headers on the wire.
There was a problem hiding this comment.
Switched to getSetCookie() — iterates each cookie individually via for...of and appends separately.
- Move AuthKitMiddlewareOptions to shared types.ts to break
bidirectional dependency between middleware.ts and middleware-body.ts
- Use getSetCookie() instead of get('Set-Cookie') to correctly handle
cookies containing commas
- Fix saveSession mock shape to match production code (destructures
{ response } not { headers }) and assert Set-Cookie reaches response
- Update oxlint error message to include middleware-body.ts as target
2 participants
Summary
Fixes #82 —
SyntaxError: eventemitter3 does not provide an export named 'default'whenauthkitMiddleware()is added tostart.tsin Vite dev.Same class of leak as #72 (fixed in #75), but in a different code path:
actions.ts/server-functions.ts—createServerFnhandlers get rewritten tocreateClientRpcstubs by the TanStack compiler, so dynamic imports in handler bodies are dead-code-eliminated from the client graph.middleware.ts—createMiddleware().server()callbacks get no such rewrite. The static import ofauthkit-loader.tspulled@workos/authkit-session→@workos-inc/node→eventemitter3(CJS) into the browser module graph during Vite dev.What changed
middleware.tsis now a thin shell thatawait import()smiddleware-body.tsinside the.server()callback. The dynamic import is unreachable from the client graph.middleware.tsto theno-restricted-importsoverride and./middleware-body*to the forbidden patterns.eventemitter3tocheck-bundle-leak.sh.middleware.spec.ts, logic tests inmiddleware-body.spec.ts.Test plan
pnpm test— 219 tests pass (18 files)pnpm build— passes (includes typecheck)pnpm lint— 0 warnings, 0 errorspnpm run build:check— no server-side fingerprints in client bundleimport { getAuthkit } from './authkit-loader.js'tomiddleware.ts→ lint error