diff --git a/.changeset/lazy-platform-compilation.md b/.changeset/lazy-platform-compilation.md new file mode 100644 index 000000000..5da2d2068 --- /dev/null +++ b/.changeset/lazy-platform-compilation.md @@ -0,0 +1,8 @@ +--- +"@callstack/repack": minor +--- + +Bring the Rspack development experience in line with Webpack by compiling each +platform only when its bundle is first requested. Multi-platform development +servers no longer eagerly build unused platforms, so launching an iOS app does +not wait for Android to compile, and vice versa. diff --git a/apps/tester-app/__tests__/lazy-compilation.test.ts b/apps/tester-app/__tests__/lazy-compilation.test.ts new file mode 100644 index 000000000..c7a513a33 --- /dev/null +++ b/apps/tester-app/__tests__/lazy-compilation.test.ts @@ -0,0 +1,106 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import rspackCommands from '@callstack/repack/commands/rspack'; +import { MultiCompiler } from '@rspack/core'; +import getPort from 'get-port'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +const TMP_DIR = path.join(__dirname, 'out/lazy-compilation'); + +let port: number; +let stopServer: (() => Promise) | undefined; + +describe('lazy compilation', () => { + const startCommand = rspackCommands.find( + (command) => command.name === 'start' + ); + if (!startCommand) throw new Error('start command not found'); + + const getStats = (platform: string) => + fetch(`http://localhost:${port}/api/${platform}/stats`).then((response) => + response.json() + ); + + beforeAll(async () => { + await fs.promises.rm(TMP_DIR, { recursive: true, force: true }); + + port = await getPort(); + + const config = { + root: path.join(__dirname, '..'), + platforms: { ios: {}, android: {} }, + reactNativePath: path.join(__dirname, '../node_modules/react-native'), + }; + + const args = { + port, + // No `platform` arg — both ios and android are configured, + // which enables the lazy compilation watchRun gate mechanism. + logFile: path.join(TMP_DIR, 'server.log'), + webpackConfig: path.join(__dirname, 'configs', './rspack.config.mjs'), + }; + + // @ts-ignore + const { stop } = await startCommand.func([], config, args); + stopServer = stop; + }); + + afterAll(async () => { + if (stopServer) { + await stopServer(); + } + }); + + it( + 'compiles each platform when its bundle is first requested', + async () => { + const [initialIosStats, initialAndroidStats] = await Promise.all([ + getStats('ios'), + getStats('android'), + ]); + expect(initialIosStats.data).toBeNull(); + expect(initialAndroidStats.data).toBeNull(); + + const iosResponse = await fetch( + `http://localhost:${port}/index.bundle?platform=ios` + ); + await iosResponse.text(); + expect(iosResponse.status).toBe(200); + + const [iosStats, androidStats] = await Promise.all([ + getStats('ios'), + getStats('android'), + ]); + expect(iosStats.data).not.toBeNull(); + expect(androidStats.data).toBeNull(); + + const androidResponse = await fetch( + `http://localhost:${port}/index.bundle?platform=android` + ); + await androidResponse.text(); + expect(androidResponse.status).toBe(200); + const finalAndroidStats = await getStats('android'); + expect(finalAndroidStats.data).not.toBeNull(); + }, + 60 * 1000 + ); + + it('stops the dev server when compiler shutdown fails', async () => { + const stop = stopServer; + if (!stop) throw new Error('Dev server was not started'); + + const close = MultiCompiler.prototype.close; + const closeError = new Error('close failed'); + MultiCompiler.prototype.close = function (callback) { + close.call(this, () => callback(closeError)); + }; + + try { + await expect(stop()).rejects.toBe(closeError); + await expect(fetch(`http://localhost:${port}/status`)).rejects.toThrow(); + stopServer = undefined; + } finally { + MultiCompiler.prototype.close = close; + } + }); +}); diff --git a/packages/repack/src/commands/rspack/Compiler.ts b/packages/repack/src/commands/rspack/Compiler.ts index fc769d9bf..7bf25727c 100644 --- a/packages/repack/src/commands/rspack/Compiler.ts +++ b/packages/repack/src/commands/rspack/Compiler.ts @@ -4,6 +4,7 @@ import type { SendProgress, Server } from '@callstack/repack-dev-server'; import type { MultiCompiler, MultiRspackOptions, + Compiler as RspackCompiler, StatsCompilation, } from '@rspack/core'; import { rspack } from '@rspack/core'; @@ -23,16 +24,22 @@ export class Compiler implements CompilerInterface { statsCache: Record = {}; resolvers: Record void>> = {}; progressSenders: Record = {}; - isCompilationInProgress = false; + isCompilationInProgress: Record = {}; // late-init devServerContext!: Server.DelegateContext; + private pendingCompilations = new Map void>(); + private activePlatforms = new Set(); + private isClosed = false; + constructor( configs: MultiRspackOptions, private reporter: Reporter, private rootDir: string ) { const handler = (platform: string, value: number) => { + if (!this.activePlatforms.has(platform)) return; + const percentage = Math.floor(value * 100); this.progressSenders[platform]?.forEach((sendProgress) => { sendProgress({ completed: percentage, total: 100 }); @@ -60,7 +67,9 @@ export class Compiler implements CompilerInterface { // @ts-expect-error memfs is compatible enough this.compiler.outputFileSystem = this.filesystem; - this.setupCompiler(); + for (const childCompiler of this.compiler.compilers) { + this.setupChildCompilerHooks(childCompiler); + } } get devServerOptions() { @@ -95,40 +104,64 @@ export class Compiler implements CompilerInterface { this.devServerContext = ctx; } - private setupCompiler() { - this.compiler.hooks.watchRun.tap('repack:watch', () => { - this.isCompilationInProgress = true; - this.platforms.forEach((platform) => { - if (platform === 'android') { - void runAdbReverse({ - port: this.devServerContext.options.port, - logger: this.devServerContext.log, - }); + private setupChildCompilerHooks(childCompiler: RspackCompiler) { + const platform = childCompiler.options.name!; + + childCompiler.hooks.watchRun.tapAsync( + 'repack:lazy-compilation', + (_compiler, done) => { + if (this.activePlatforms.has(platform)) { + done(); + return; } - this.devServerContext.notifyBuildStart(platform); - this.devServerContext.broadcastToHmrClients({ - action: 'compiling', - body: { name: platform }, + + this.pendingCompilations.set(platform, () => { + // Exclude time spent waiting for the platform to be requested and + // avoid replaying file changes that happened before that request. + if (childCompiler.watching) { + const startTime = Date.now(); + childCompiler.watching.startTime = startTime; + childCompiler.watching.lastWatcherStartTime = startTime; + } + done(); + }); + } + ); + + childCompiler.hooks.watchRun.tap('repack:watch', () => { + if (!this.activePlatforms.has(platform)) return; + + this.isCompilationInProgress[platform] = true; + + if (platform === 'android') { + void runAdbReverse({ + port: this.devServerContext.options.port, + logger: this.devServerContext.log, }); + } + + this.devServerContext.notifyBuildStart(platform); + this.devServerContext.broadcastToHmrClients({ + action: 'compiling', + body: { name: platform }, }); }); - this.compiler.hooks.invalid.tap('repack:invalid', () => { - this.isCompilationInProgress = true; - this.platforms.forEach((platform) => { - this.devServerContext.notifyBuildStart(platform); - this.devServerContext.broadcastToHmrClients({ - action: 'compiling', - body: { name: platform }, - }); + childCompiler.hooks.invalid.tap('repack:invalid', () => { + if (!this.activePlatforms.has(platform)) return; + + this.isCompilationInProgress[platform] = true; + this.devServerContext.notifyBuildStart(platform); + this.devServerContext.broadcastToHmrClients({ + action: 'compiling', + body: { name: platform }, }); }); - this.compiler.hooks.done.tap('repack:done', (multiStats) => { - const stats = multiStats.toJson({ + childCompiler.hooks.done.tap('repack:done', (stats) => { + const childStats = stats.toJson({ all: false, assets: true, - children: true, outputPath: true, timings: true, hash: true, @@ -137,55 +170,52 @@ export class Compiler implements CompilerInterface { }); try { - stats.children!.forEach((childStats) => { - const platform = childStats.name!; - this.devServerContext.broadcastToHmrClients({ - action: 'hash', - body: { name: platform, hash: childStats.hash }, - }); - - this.statsCache[platform] = childStats; - const assets = childStats.assets!; - - this.assetsCache[platform] = assets - .filter((asset) => asset.type === 'asset') - .reduce( - (acc, { name, info, size }) => { - const assetPath = path.join(childStats.outputPath!, name); - const data = this.filesystem.readFileSync(assetPath) as Buffer; - const asset = { data, info, size }; - - acc[adaptFilenameToPlatform(name)] = asset; - - if (info.related?.sourceMap) { - const sourceMapName = Array.isArray(info.related.sourceMap) - ? info.related.sourceMap[0] - : info.related.sourceMap; - const sourceMapPath = path.join( - childStats.outputPath!, - sourceMapName - ); - const sourceMapData = this.filesystem.readFileSync( - sourceMapPath - ) as Buffer; - const sourceMapAsset = { - data: sourceMapData, - info: { - hotModuleReplacement: info.hotModuleReplacement, - size: sourceMapData.length, - }, - size: sourceMapData.length, - }; - - acc[adaptFilenameToPlatform(sourceMapName)] = sourceMapAsset; - } - - return acc; - }, - // keep old assets - this.assetsCache[platform] ?? {} - ); + this.devServerContext.broadcastToHmrClients({ + action: 'hash', + body: { name: platform, hash: childStats.hash }, }); + + this.statsCache[platform] = childStats; + const assets = childStats.assets!; + + this.assetsCache[platform] = assets + .filter((asset) => asset.type === 'asset') + .reduce( + (acc, { name, info, size }) => { + const assetPath = path.join(childStats.outputPath!, name); + const data = this.filesystem.readFileSync(assetPath) as Buffer; + const asset = { data, info, size }; + + acc[adaptFilenameToPlatform(name)] = asset; + + if (info.related?.sourceMap) { + const sourceMapName = Array.isArray(info.related.sourceMap) + ? info.related.sourceMap[0] + : info.related.sourceMap; + const sourceMapPath = path.join( + childStats.outputPath!, + sourceMapName + ); + const sourceMapData = this.filesystem.readFileSync( + sourceMapPath + ) as Buffer; + const sourceMapAsset = { + data: sourceMapData, + info: { + hotModuleReplacement: info.hotModuleReplacement, + size: sourceMapData.length, + }, + size: sourceMapData.length, + }; + + acc[adaptFilenameToPlatform(sourceMapName)] = sourceMapAsset; + } + + return acc; + }, + // keep old assets + this.assetsCache[platform] ?? {} + ); } catch (error) { this.reporter.process({ type: 'error', @@ -198,27 +228,38 @@ export class Compiler implements CompilerInterface { }); } - this.isCompilationInProgress = false; + this.isCompilationInProgress[platform] = false; + this.callPendingResolvers(platform); - stats.children?.forEach((childStats) => { - const platform = childStats.name!; - const time = childStats.time!; - this.callPendingResolvers(platform); - this.devServerContext.notifyBuildEnd(platform); - this.devServerContext.broadcastToHmrClients({ - action: 'ok', - body: { name: platform }, - }); - this.reporter.process({ - issuer: 'DevServer', - message: [{ progress: { platform, time } }], - timestamp: Date.now(), - type: 'progress', - }); + this.devServerContext.notifyBuildEnd(platform); + this.devServerContext.broadcastToHmrClients({ + action: 'ok', + body: { name: platform }, + }); + this.reporter.process({ + issuer: 'DevServer', + message: [{ progress: { platform, time: childStats.time } }], + timestamp: Date.now(), + type: 'progress', }); }); } + private activatePlatform(platform: string) { + if (!this.platforms.includes(platform)) { + throw new CLIError(`Unrecognized platform: ${platform}`); + } + if (this.activePlatforms.has(platform)) return; + this.activePlatforms.add(platform); + this.isCompilationInProgress[platform] = true; + + const resumeCompilation = this.pendingCompilations.get(platform); + if (resumeCompilation) { + this.pendingCompilations.delete(platform); + resumeCompilation(); + } + } + start() { this.compiler.watch(this.watchOptions, (error) => { if (!error) return; @@ -228,11 +269,32 @@ export class Compiler implements CompilerInterface { }); } + close(callback: (error?: Error | null) => void = () => {}) { + this.isClosed = true; + const error = new Error('Compiler closed before compilation completed'); + this.platforms.forEach((platform) => { + this.callPendingResolvers(platform, error); + }); + + // Resume pending compilations so Watching instances can close cleanly + for (const resumeCompilation of this.pendingCompilations.values()) { + resumeCompilation(); + } + this.pendingCompilations.clear(); + this.compiler.close(callback); + } + async getAsset( filename: string, platform: string, sendProgress?: SendProgress ): Promise { + if (this.isClosed) { + throw new Error('Compiler closed before compilation completed'); + } + + this.activatePlatform(platform); + // Return file from assetsCache if exists const fileFromCache = this.assetsCache[platform]?.[filename]; if (fileFromCache) { @@ -241,7 +303,7 @@ export class Compiler implements CompilerInterface { this.addProgressSender(platform, sendProgress); - if (!this.isCompilationInProgress) { + if (!this.isCompilationInProgress[platform]) { this.removeProgressSender(platform, sendProgress); return Promise.reject( new Error( diff --git a/packages/repack/src/commands/rspack/__tests__/Compiler.test.ts b/packages/repack/src/commands/rspack/__tests__/Compiler.test.ts new file mode 100644 index 000000000..26e1ff7e5 --- /dev/null +++ b/packages/repack/src/commands/rspack/__tests__/Compiler.test.ts @@ -0,0 +1,178 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import type { Server } from '@callstack/repack-dev-server'; +import type { MultiRspackOptions } from '@rspack/core'; +import type { Reporter } from '../../../logging/types.js'; +import { Compiler } from '../Compiler.js'; + +// Mock adb reverse to avoid calling adb during tests +jest.mock('../../common/runAdbReverse.js', () => ({ + runAdbReverse: jest.fn().mockResolvedValue(undefined), +})); + +describe('Compiler – lazy compilation', () => { + let tmpDir: string; + let entryPath: string; + const compilationCounts = { ios: 0, android: 0 }; + + const reporter: Reporter = { + process: jest.fn(), + flush: jest.fn(), + stop: jest.fn(), + }; + + const mockDevServerContext: Server.DelegateContext = { + options: { port: 8081 } as Server.DelegateContext['options'], + log: { + warn: jest.fn(), + error: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + } as unknown as Server.DelegateContext['log'], + notifyBuildStart: jest.fn(), + notifyBuildEnd: jest.fn(), + broadcastToHmrClients: jest.fn(), + broadcastToMessageClients: jest.fn(), + }; + + beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'repack-compiler-test-')); + entryPath = path.join(tmpDir, 'entry.js'); + fs.writeFileSync(entryPath, 'module.exports = {};'); + }); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function createConfigs(): MultiRspackOptions { + return [ + { + name: 'ios', + mode: 'development', + entry: entryPath, + output: { filename: 'main.js', path: path.join(tmpDir, 'out-ios') }, + plugins: [], + watchOptions: { poll: 10 }, + }, + { + name: 'android', + mode: 'development', + entry: entryPath, + output: { + filename: 'main.js', + path: path.join(tmpDir, 'out-android'), + }, + plugins: [], + watchOptions: { poll: 10 }, + }, + ]; + } + + describe('platform activation', () => { + let compiler: Compiler; + + beforeAll(() => { + compiler = new Compiler(createConfigs(), reporter, tmpDir); + compiler.setDevServerContext(mockDevServerContext); + for (const childCompiler of compiler.compiler.compilers) { + const platform = childCompiler.options + .name as keyof typeof compilationCounts; + childCompiler.hooks.done.tap('test:count-builds', () => { + compilationCounts[platform]++; + }); + } + compiler.start(); + }); + + afterAll(async () => { + await new Promise((resolve, reject) => { + compiler.close((error) => (error ? reject(error) : resolve())); + }); + }); + + it('rejects unconfigured platforms', async () => { + await expect( + compiler.getAsset('main.js', 'windows') + ).rejects.toThrowError('Unrecognized platform: windows'); + expect(compilationCounts).toEqual({ ios: 0, android: 0 }); + }); + + it('compiles each platform on demand and reuses cached assets', async () => { + // Change source after both watchers are gated, then let polling observe it. + await new Promise((resolve) => setTimeout(resolve, 100)); + fs.writeFileSync(entryPath, 'module.exports = { updated: true };'); + await new Promise((resolve) => setTimeout(resolve, 100)); + const iosAsset = await compiler.getAsset('main.js', 'ios'); + // Give polling time to trigger any stale-timestamp rebuild. + await new Promise((resolve) => setTimeout(resolve, 200)); + + expect(iosAsset.data).toBeInstanceOf(Buffer); + expect(compiler.statsCache.ios).toBeDefined(); + expect(compiler.statsCache.android).toBeUndefined(); + expect(compilationCounts).toEqual({ ios: 1, android: 0 }); + + const androidAsset = await compiler.getAsset('main.js', 'android'); + + expect(androidAsset.data).toBeInstanceOf(Buffer); + expect(compiler.statsCache.android).toBeDefined(); + expect(compilationCounts).toEqual({ ios: 1, android: 1 }); + + const cachedAsset = await compiler.getAsset('main.js', 'ios'); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(cachedAsset.data).toBeInstanceOf(Buffer); + expect(compilationCounts).toEqual({ ios: 1, android: 1 }); + }); + }); + + describe('close()', () => { + it('rejects pending asset requests', async () => { + const compiler = new Compiler(createConfigs(), reporter, tmpDir); + compiler.setDevServerContext(mockDevServerContext); + compiler.compiler.compilers[0].hooks.make.tapAsync( + 'test:hold-compilation', + (_compilation, done) => setTimeout(done, 100) + ); + compiler.start(); + + const assetRequest = expect( + compiler.getAsset('main.js', 'ios') + ).rejects.toThrow('Compiler closed before compilation completed'); + await new Promise((resolve, reject) => { + compiler.close((error) => (error ? reject(error) : resolve())); + }); + + await assetRequest; + await expect(compiler.getAsset('main.js', 'android')).rejects.toThrow( + 'Compiler closed before compilation completed' + ); + }); + + it('resolves when both platform compilations are pending', async () => { + const compiler = new Compiler(createConfigs(), reporter, tmpDir); + compiler.setDevServerContext(mockDevServerContext); + compiler.start(); + + // Both platform compilations are pending — close() should resume them + await new Promise((resolve, reject) => { + compiler.close((error) => (error ? reject(error) : resolve())); + }); + }); + + it('forwards compiler close errors to the caller', async () => { + const compiler = new Compiler(createConfigs(), reporter, tmpDir); + const closeError = new Error('close failed'); + jest + .spyOn(compiler.compiler, 'close') + .mockImplementation((callback) => callback(closeError)); + + await expect( + new Promise((resolve, reject) => { + compiler.close((error) => (error ? reject(error) : resolve())); + }) + ).rejects.toBe(closeError); + }); + }); +}); diff --git a/packages/repack/src/commands/rspack/types.ts b/packages/repack/src/commands/rspack/types.ts deleted file mode 100644 index 9207803ac..000000000 --- a/packages/repack/src/commands/rspack/types.ts +++ /dev/null @@ -1,3 +0,0 @@ -import type { MultiCompiler } from '@rspack/core'; - -export type MultiWatching = ReturnType; diff --git a/packages/repack/src/commands/start.ts b/packages/repack/src/commands/start.ts index b67d85310..ef1f232b0 100644 --- a/packages/repack/src/commands/start.ts +++ b/packages/repack/src/commands/start.ts @@ -253,7 +253,13 @@ export async function start( return { stop: async () => { reporter.stop(); - await stop(); + try { + await new Promise((resolve, reject) => { + compiler.close((error) => (error ? reject(error) : resolve())); + }); + } finally { + await stop(); + } }, }; } diff --git a/packages/repack/src/commands/types.ts b/packages/repack/src/commands/types.ts index f06a26885..ee98126e6 100644 --- a/packages/repack/src/commands/types.ts +++ b/packages/repack/src/commands/types.ts @@ -111,6 +111,7 @@ export interface CompilerInterface { statsCache: Record; setDevServerContext(ctx: Server.DelegateContext): void; start(): void; + close(callback?: (error?: Error | null) => void): void; getAsset( filename: string, platform: string, diff --git a/packages/repack/src/commands/webpack/Compiler.ts b/packages/repack/src/commands/webpack/Compiler.ts index d01be1810..76d27b086 100644 --- a/packages/repack/src/commands/webpack/Compiler.ts +++ b/packages/repack/src/commands/webpack/Compiler.ts @@ -47,6 +47,17 @@ export class Compiler implements CompilerInterface { // no-op: webpack workers spawn lazily on first getAsset call } + close(callback: (error?: Error | null) => void = () => {}) { + void Promise.all( + Object.values(this.workers).map((worker) => worker.terminate()) + ).then( + () => callback(), + (error: unknown) => { + callback(error instanceof Error ? error : new Error(String(error))); + } + ); + } + private spawnWorker(platform: string) { this.isCompilationInProgress[platform] = true; diff --git a/packages/repack/src/commands/webpack/__tests__/Compiler.test.ts b/packages/repack/src/commands/webpack/__tests__/Compiler.test.ts index e75b794d1..b89debb99 100644 --- a/packages/repack/src/commands/webpack/__tests__/Compiler.test.ts +++ b/packages/repack/src/commands/webpack/__tests__/Compiler.test.ts @@ -12,6 +12,7 @@ jest.mock('node:worker_threads', () => { Object.assign(new EventEmitter(), { stdout: new EventEmitter(), stderr: new EventEmitter(), + terminate: jest.fn(() => Promise.resolve(0)), }) ), }; @@ -40,3 +41,32 @@ test('rejects a pending asset request when the worker reports an error', async ( expect(compiler.resolvers.ios).toHaveLength(0); await expect(request).rejects.toBe(error); }); + +test('terminates active workers when closed', async () => { + const reporter: Reporter = { + process: jest.fn(), + flush: jest.fn(), + stop: jest.fn(), + }; + const compiler = new Compiler( + ['ios'], + { host: '' }, + reporter, + '/project', + '/react-native' + ); + const worker = new Worker('worker.js'); + compiler.workers.ios = worker; + + await new Promise((resolve, reject) => { + compiler.close((error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); + + expect(worker.terminate).toHaveBeenCalledTimes(1); +});