diff --git a/.changepacks/changepack_log_pnNusktwcvu2vXQOHlpxd.json b/.changepacks/changepack_log_pnNusktwcvu2vXQOHlpxd.json new file mode 100644 index 00000000..cc0eea91 --- /dev/null +++ b/.changepacks/changepack_log_pnNusktwcvu2vXQOHlpxd.json @@ -0,0 +1 @@ +{"changes":{"packages/next-plugin/package.json":"Patch"},"note":"Fix write issue","date":"2026-05-13T07:13:54.316827100Z"} \ No newline at end of file diff --git a/.gitignore b/.gitignore index ebc6e373..76137b57 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,4 @@ storybook-static test-results playwright-report .omc +.playwright-mcp diff --git a/packages/next-plugin/src/__tests__/coordinator.test.ts b/packages/next-plugin/src/__tests__/coordinator.test.ts index 02c7f4b8..711540aa 100644 --- a/packages/next-plugin/src/__tests__/coordinator.test.ts +++ b/packages/next-plugin/src/__tests__/coordinator.test.ts @@ -15,6 +15,7 @@ import { import { type CoordinatorOptions, + flushCoordinatorWrites, resetCoordinator, startCoordinator, } from '../coordinator' @@ -481,14 +482,14 @@ describe('coordinator', () => { const portStr = (writeFileSyncSpy.mock.calls[0] as [string, string])[1] const port = parseInt(portStr) - // Mock Date.now to simulate time passing beyond MAX_WAIT_MS (30s) + // Mock Date.now to simulate time passing beyond MAX_WAIT_MS (60s). let callCount = 0 const dateNowSpy = spyOn(Date, 'now').mockImplementation(() => { callCount++ - // First call is `const start = Date.now()` → return 0 + // First call is `const start = Date.now()` — return 0 // Subsequent calls return past MAX_WAIT_MS threshold if (callCount <= 1) return 0 - return 31_000 + return 61_000 }) // Request CSS with waitForIdle=true but no extractions ever happen @@ -712,4 +713,240 @@ describe('coordinator', () => { // Calling again should be safe (server is already null) resetCoordinator() }) + + it('should coalesce duplicate writes to the same path within one /extract handler', async () => { + // When a single /extract invocation triggers multiple writes to the same + // path (singleCss + updatedBaseStyle=true → both the base-CSS write and + // the cssFile write target `devup-ui.css`), the second write must be + // collapsed by the latest-wins serializer: the first chained run sees the + // *latest* content and writes it once, the second chained run finds + // `latestContent` already consumed and resolves as a no-op. + codeExtractSpy.mockReturnValue({ + code: 'single css code', + map: undefined, + cssFile: 'devup-ui.css', + updatedBaseStyle: true, + free: mock(), + [Symbol.dispose]: mock(), + }) + getCssSpy.mockReturnValue('all-styles') + exportSheetSpy.mockReturnValue('sheet-json') + exportClassMapSpy.mockReturnValue('classmap-json') + exportFileMapSpy.mockReturnValue('filemap-json') + + const options = makeOptions({ singleCss: true }) + const coordinator = startCoordinator(options) + + await new Promise((r) => setTimeout(r, 100)) + + const portStr = (writeFileSyncSpy.mock.calls[0] as [string, string])[1] + const port = parseInt(portStr) + + const res = await httpRequest( + port, + 'POST', + '/extract', + JSON.stringify({ + filename: 'src/App.tsx', + code: 'const x = ', + resourcePath: join(process.cwd(), 'src', 'App.tsx'), + }), + ) + + expect(res.status).toBe(200) + + // Both safeWrite calls target `devup-ui.css`. Coalescing means exactly + // one physical writeFile call is made for that path; the sheet/classMap/ + // fileMap writes (3 more) all go to distinct paths. + const devupUiCssWrites = writeFileSpy.mock.calls.filter((call) => + String(call[0]).endsWith('devup-ui.css'), + ) + expect(devupUiCssWrites.length).toBe(1) + + coordinator.close() + }) + + it('should expose flushCoordinatorWrites to drain queued writes', async () => { + // The exported helper must return a settled promise even when no writes + // are pending (idle coordinator), so build orchestration can safely await + // it without risk of hanging. + await expect(flushCoordinatorWrites()).resolves.toBeUndefined() + + // After triggering a real /extract, awaiting the helper must wait for all + // queued writes (chained per path) to settle. We assert the spy has been + // invoked by the time the helper resolves. + codeExtractSpy.mockReturnValue({ + code: 'code', + map: undefined, + cssFile: 'devup-ui-7.css', + updatedBaseStyle: false, + free: mock(), + [Symbol.dispose]: mock(), + }) + getCssSpy.mockReturnValue('per-file-css') + exportSheetSpy.mockReturnValue('sheet-json') + exportClassMapSpy.mockReturnValue('classmap-json') + exportFileMapSpy.mockReturnValue('filemap-json') + + const options = makeOptions() + const coordinator = startCoordinator(options) + + await new Promise((r) => setTimeout(r, 100)) + + const portStr = (writeFileSyncSpy.mock.calls[0] as [string, string])[1] + const port = parseInt(portStr) + + await httpRequest( + port, + 'POST', + '/extract', + JSON.stringify({ + filename: 'src/App.tsx', + code: 'const x = ', + resourcePath: join(process.cwd(), 'src', 'App.tsx'), + }), + ) + + await expect(flushCoordinatorWrites()).resolves.toBeUndefined() + expect(writeFileSpy.mock.calls.length).toBeGreaterThan(0) + + coordinator.close() + }) + + it('should continue chained writes after a previous write fails (chain error recovery)', async () => { + // The serializer must not let one failed write poison every subsequent + // write for that path. We force the first writeFile to fail, then verify + // the second extraction's writes still happen for that same path. + codeExtractSpy.mockReturnValue({ + code: 'code', + map: undefined, + cssFile: 'devup-ui.css', + updatedBaseStyle: false, + free: mock(), + [Symbol.dispose]: mock(), + }) + getCssSpy.mockReturnValue('css-content') + exportSheetSpy.mockReturnValue('sheet-json') + exportClassMapSpy.mockReturnValue('classmap-json') + exportFileMapSpy.mockReturnValue('filemap-json') + + // Re-install writeFile spy with controlled failure: any write to the + // devup-ui.css path errors out on the *first* invocation only. + writeFileSpy.mockRestore() + let devupCssCallCount = 0 + writeFileSpy = spyOn(fs, 'writeFile').mockImplementation( + (_path: any, _data: any, _encOrCb: any, maybeCb?: any) => { + const cb = typeof _encOrCb === 'function' ? _encOrCb : maybeCb + if (cb) { + if (String(_path).endsWith('devup-ui.css')) { + devupCssCallCount++ + if (devupCssCallCount === 1) { + cb(new Error('simulated disk error')) + return + } + } + cb(null) + } + }, + ) + + const options = makeOptions({ singleCss: true }) + const coordinator = startCoordinator(options) + + await new Promise((r) => setTimeout(r, 100)) + + const portStr = (writeFileSyncSpy.mock.calls[0] as [string, string])[1] + const port = parseInt(portStr) + + // First /extract: triggers a write to devup-ui.css that we make fail. + // The coordinator will respond with 500 (await Promise.all([..., failingWrite]) + // rejects), but the chain itself must NOT be poisoned. + const firstRes = await httpRequest( + port, + 'POST', + '/extract', + JSON.stringify({ + filename: 'src/A.tsx', + code: 'const x = ', + resourcePath: join(process.cwd(), 'src', 'A.tsx'), + }), + ) + expect(firstRes.status).toBe(500) + + // Second /extract for the same path must SUCCEED — the `.catch(() => {})` + // chain-survival branch is what makes this work. + const secondRes = await httpRequest( + port, + 'POST', + '/extract', + JSON.stringify({ + filename: 'src/B.tsx', + code: 'const y = ', + resourcePath: join(process.cwd(), 'src', 'B.tsx'), + }), + ) + expect(secondRes.status).toBe(200) + + // We must have observed at least 2 attempts on the devup-ui.css path: + // the first (failed) and the second (succeeded). + expect(devupCssCallCount).toBeGreaterThanOrEqual(2) + + coordinator.close() + }) + + it('should release the pending-extract slot when readBody throws before promotion', async () => { + // If JSON.parse on the request body throws, the handler must still tear + // down its `pendingExtractStarts` reservation (rather than the active + // counter) so waitForIdle is not left waiting forever for a phantom + // extraction. We verify by sending a malformed body, then proving the + // coordinator still processes a follow-up extraction normally. + codeExtractSpy.mockReturnValue({ + code: 'code', + map: undefined, + cssFile: 'devup-ui-1.css', + updatedBaseStyle: false, + free: mock(), + [Symbol.dispose]: mock(), + }) + getCssSpy.mockReturnValue('per-file-css') + exportSheetSpy.mockReturnValue('sheet-json') + exportClassMapSpy.mockReturnValue('classmap-json') + exportFileMapSpy.mockReturnValue('filemap-json') + + const options = makeOptions() + const coordinator = startCoordinator(options) + + await new Promise((r) => setTimeout(r, 100)) + + const portStr = (writeFileSyncSpy.mock.calls[0] as [string, string])[1] + const port = parseInt(portStr) + + // Send an invalid body so JSON.parse throws BEFORE activeExtractions is + // incremented. The handler must still respond 500 cleanly. + const badRes = await httpRequest(port, 'POST', '/extract', 'not-json') + expect(badRes.status).toBe(500) + const errorPayload = JSON.parse(badRes.body) as { error: string } + expect(typeof errorPayload.error).toBe('string') + + // A subsequent well-formed extraction must succeed. If the pending-slot + // bookkeeping was wrong (decrementing activeExtractions instead of + // pendingExtractStarts in finally), internal counters would drift negative + // — that would not crash this request but is asserted by the next test + // case via waitForIdle behaviour. + const goodRes = await httpRequest( + port, + 'POST', + '/extract', + JSON.stringify({ + filename: 'src/App.tsx', + code: 'const x = ', + resourcePath: join(process.cwd(), 'src', 'App.tsx'), + }), + ) + expect(goodRes.status).toBe(200) + const okPayload = JSON.parse(goodRes.body) as { code: string } + expect(okPayload.code).toBe('code') + + coordinator.close() + }) }) diff --git a/packages/next-plugin/src/coordinator.ts b/packages/next-plugin/src/coordinator.ts index 1c79a790..4719e511 100644 --- a/packages/next-plugin/src/coordinator.ts +++ b/packages/next-plugin/src/coordinator.ts @@ -22,6 +22,66 @@ export interface CoordinatorOptions { coordinatorPortFile: string } +// Latest-Wins Coalescing Serializer. +// +// Multiple Turbopack workers may call /extract concurrently, each producing +// CSS for the same target file (especially `devup-ui.css` in singleCss mode +// where every file writes to it). Naive `writeFile` calls run in parallel via +// libuv's thread pool with no completion-order guarantees, so a stale snapshot +// can clobber a fresher one — leaving the on-disk CSS missing rules whose +// class names already landed in the JSX markup. +// +// `safeWrite` solves this with a per-path FIFO chain + content coalescing: +// 1. Each call records the latest content for the path (overwrites earlier). +// 2. The next disk write is chained after the previous one for the same +// path, guaranteeing serial execution in invocation order. +// 3. When the chained write actually runs, it pulls the most recent content +// (not the original captured value), so intermediate snapshots between +// enqueue-time and run-time are coalesced into a single write. +// +// Net effect: race becomes mathematically impossible (single-threaded JS + +// FIFO queue), and total disk IO drops dramatically because N stale snapshots +// for the same file are collapsed into 1 effective write. +const writeChain = new Map>() +const latestContent = new Map() + +function safeWrite(path: string, content: string): Promise { + // Always record the most recent content for this path so a queued write + // picks up the latest snapshot when it runs. + latestContent.set(path, content) + + // Swallow any prior error solely for chaining purposes — the actual caller + // that hit the error already saw it via the returned promise, but we must + // not let one failure poison every subsequent write for this path. + const prev = (writeChain.get(path) ?? Promise.resolve()).catch(() => {}) + + const next = prev.then( + () => + new Promise((resolve, reject) => { + const final = latestContent.get(path) + if (final === undefined) { + // An earlier chained run already consumed the latest content for + // this path; nothing new to write. Resolve as a no-op. + resolve() + return + } + latestContent.delete(path) + writeFile(path, final, 'utf-8', (err) => + err ? reject(err) : resolve(), + ) + }), + ) + + writeChain.set(path, next) + return next +} + +// Best-effort drain of every pending write. Used on coordinator close so the +// build process does not exit with stale files mid-flight. +function flushPendingWrites(): Promise { + return Promise.allSettled([...writeChain.values()]).then(() => undefined) +} + function readBody(req: IncomingMessage): Promise { return new Promise((resolve, reject) => { const chunks: Buffer[] = [] @@ -33,12 +93,31 @@ function readBody(req: IncomingMessage): Promise { let server: Server | null = null -// Extraction tracking for waitForIdle +// Extraction tracking for waitForIdle. +// +// The CSS loader fetches the live sheet from `/css?waitForIdle=true`. In +// production builds the response of that call is what Turbopack bundles +// for the `devup-ui.css` module — there is no second chance. So waitForIdle +// must NOT resolve while there are still extractions in flight, or that +// have not yet started but will start soon. +// +// We track two complementary signals: +// * activeExtractions / lastCompletedAt → are extractions currently happening? +// * pendingExtractStarts → did anyone POST /extract that hasn't progressed +// to `activeExtractions++` yet (e.g. still inside `await readBody(req)`)? +// +// IDLE_THRESHOLD_MS is increased so an early `/css` request — triggered by +// the first .tsx loader resolving its import graph before the rest of the +// route's files are processed — cannot resolve in the gap between two +// extraction batches. Empirically the gap between Turbopack extraction +// "waves" in a 64-route landing build can exceed the previous 500ms, which +// caused the snapshot to capture only the early routes' styles. let activeExtractions = 0 let totalExtractions = 0 let lastCompletedAt = 0 -const IDLE_THRESHOLD_MS = 500 -const MAX_WAIT_MS = 30_000 +let pendingExtractStarts = 0 +const IDLE_THRESHOLD_MS = 2500 +const MAX_WAIT_MS = 60_000 function waitForIdle(): Promise { const start = Date.now() @@ -46,13 +125,14 @@ function waitForIdle(): Promise { const check = () => { const now = Date.now() if (now - start > MAX_WAIT_MS) { - // Timeout — return whatever CSS we have + // Hard timeout — give up and return whatever we have. resolve() return } if ( totalExtractions > 0 && activeExtractions === 0 && + pendingExtractStarts === 0 && now - lastCompletedAt >= IDLE_THRESHOLD_MS ) { resolve() @@ -103,9 +183,18 @@ export function startCoordinator(options: CoordinatorOptions): { } if (req.method === 'POST' && url.pathname === '/extract') { - activeExtractions++ + // Reserve a "start slot" before yielding on `await readBody`. Without + // this counter, `waitForIdle` could observe activeExtractions=0 in the + // window between the request hitting this handler and `activeExtractions++` + // below — making it falsely conclude the build is idle even though + // more extractions are imminent. + pendingExtractStarts++ + let promotedToActive = false try { const body = JSON.parse(await readBody(req)) + activeExtractions++ + pendingExtractStarts-- + promotedToActive = true const { filename, code, resourcePath } = body as { filename: string code: string @@ -145,13 +234,9 @@ export function startCoordinator(options: CoordinatorOptions): { if (result.updatedBaseStyle) { promises.push( - new Promise((resolve, reject) => - writeFile( - join(cssDir, 'devup-ui.css'), - `${getCss(null, false)}\n/* ${Date.now()} */`, - 'utf-8', - (err) => (err ? reject(err) : resolve()), - ), + safeWrite( + join(cssDir, 'devup-ui.css'), + `${getCss(null, false)}\n/* ${Date.now()} */`, ), ) } @@ -159,29 +244,13 @@ export function startCoordinator(options: CoordinatorOptions): { if (result.cssFile) { const fileNum = getFileNumByFilename(result.cssFile) promises.push( - new Promise((resolve, reject) => - writeFile( - join(cssDir, basename(result.cssFile!)), - getCss(fileNum, true), - 'utf-8', - (err) => (err ? reject(err) : resolve()), - ), - ), - new Promise((resolve, reject) => - writeFile(sheetFile, exportSheet(), 'utf-8', (err) => - err ? reject(err) : resolve(), - ), - ), - new Promise((resolve, reject) => - writeFile(classMapFile, exportClassMap(), 'utf-8', (err) => - err ? reject(err) : resolve(), - ), - ), - new Promise((resolve, reject) => - writeFile(fileMapFile, exportFileMap(), 'utf-8', (err) => - err ? reject(err) : resolve(), - ), + safeWrite( + join(cssDir, basename(result.cssFile)), + getCss(fileNum, true), ), + safeWrite(sheetFile, exportSheet()), + safeWrite(classMapFile, exportClassMap()), + safeWrite(fileMapFile, exportFileMap()), ) // In non-singleCss mode, imports are rewritten from devup-ui-N.css to @@ -192,13 +261,9 @@ export function startCoordinator(options: CoordinatorOptions): { // When updatedBaseStyle is true, devup-ui.css is already written above. if (!singleCss && !result.updatedBaseStyle && result.css != null) { promises.push( - new Promise((resolve, reject) => - writeFile( - join(cssDir, 'devup-ui.css'), - `${getCss(null, false)}\n/* ${Date.now()} */`, - 'utf-8', - (err) => (err ? reject(err) : resolve()), - ), + safeWrite( + join(cssDir, 'devup-ui.css'), + `${getCss(null, false)}\n/* ${Date.now()} */`, ), ) } @@ -223,7 +288,13 @@ export function startCoordinator(options: CoordinatorOptions): { }), ) } finally { - activeExtractions-- + if (promotedToActive) { + activeExtractions-- + } else { + // readBody/JSON.parse threw before we promoted to active, so the + // pending slot is still ours to release. + pendingExtractStarts-- + } totalExtractions++ lastCompletedAt = Date.now() } @@ -243,6 +314,11 @@ export function startCoordinator(options: CoordinatorOptions): { return { close: () => { + // Fire-and-forget drain of any in-flight serialized writes so the + // last-written CSS reflects the final sheet state, even though + // `close` itself returns synchronously (it is invoked from + // `process.on('exit', ...)` where awaiting is not possible). + void flushPendingWrites() if (server) { server.close() server = null @@ -256,6 +332,9 @@ export function startCoordinator(options: CoordinatorOptions): { } } +/** @internal Wait for every pending serialized write to settle. */ +export const flushCoordinatorWrites = (): Promise => flushPendingWrites() + /** @internal Reset coordinator state for testing purposes only */ export const resetCoordinator = () => { if (server) { @@ -265,4 +344,6 @@ export const resetCoordinator = () => { activeExtractions = 0 totalExtractions = 0 lastCompletedAt = 0 + writeChain.clear() + latestContent.clear() }