From a8b824790a19e4fa2176823b613c96305ac60936 Mon Sep 17 00:00:00 2001 From: Daniel Williams Date: Wed, 4 Nov 2026 13:47:30 +0000 Subject: [PATCH 1/2] fix: find hermesc under pnpm in HermesBytecodePlugin The default lookup joined `/../hermes-compiler` lexically. Under pnpm `node_modules/react-native` is a symlink into `.pnpm`, where `hermes-compiler` is installed next to the real react-native, so the join missed it and the plugin fell back to `sdks/hermesc`, which RN 0.82+ no longer ships (ENOENT when spawning hermesc). The existing two locations are still checked first, in the same order, and resolve to the same path. Only when neither exists does the lookup try `hermes-compiler` next to the symlink-resolved react-native, then the react-native `node_modules` hierarchy, then react-native resolved from the compiler context when the default `reactNativePath` doesn't exist. If nothing is found it throws an error listing every checked path. The lookup runs lazily on the first transformed asset and is cached, so builds that register the plugin but emit no matching assets are unaffected. --- .changeset/hermes-bytecode-hermesc-lookup.md | 5 + .../HermesBytecodePlugin.ts | 21 +- .../utils/getHermesCLIPath.ts | 169 ++++++++++++++- .../__tests__/HermesBytecodePlugin.test.ts | 98 +++++++-- .../__tests__/getHermesCLIPath.test.ts | 205 ++++++++++++++++++ .../src/latest/api/plugins/hermes-bytecode.md | 8 +- 6 files changed, 479 insertions(+), 27 deletions(-) create mode 100644 .changeset/hermes-bytecode-hermesc-lookup.md create mode 100644 packages/repack/src/plugins/__tests__/getHermesCLIPath.test.ts diff --git a/.changeset/hermes-bytecode-hermesc-lookup.md b/.changeset/hermes-bytecode-hermesc-lookup.md new file mode 100644 index 000000000..85e0411f7 --- /dev/null +++ b/.changeset/hermes-bytecode-hermesc-lookup.md @@ -0,0 +1,5 @@ +--- +"@callstack/repack": patch +--- + +Fix `HermesBytecodePlugin` not finding `hermesc` under pnpm (and other symlinked or nested layouts) on React Native 0.82+. The existing locations are still checked first and resolve to the same path. Only when neither exists does the plugin follow the `react-native` symlink and search its `node_modules` hierarchy for `hermes-compiler`. The lookup now runs when the first matching asset is transformed. If no binary is found, the build fails with an error listing every checked path instead of spawning a missing binary. diff --git a/packages/repack/src/plugins/HermesBytecodePlugin/HermesBytecodePlugin.ts b/packages/repack/src/plugins/HermesBytecodePlugin/HermesBytecodePlugin.ts index 227069152..9d1028c89 100644 --- a/packages/repack/src/plugins/HermesBytecodePlugin/HermesBytecodePlugin.ts +++ b/packages/repack/src/plugins/HermesBytecodePlugin/HermesBytecodePlugin.ts @@ -102,8 +102,19 @@ export class HermesBytecodePlugin { this.config.reactNativePath || path.join(compiler.context, 'node_modules', 'react-native'); - const hermesCLIPath = - this.config.hermesCLIPath || getHermesCLIPath(reactNativePath); + // Resolved on the first transformed asset, so that builds which emit no matching + // assets don't fail when hermesc can't be found. + let resolvedHermesCLIPath = this.config.hermesCLIPath; + const resolveHermesCLIPath = () => { + resolvedHermesCLIPath ||= getHermesCLIPath(reactNativePath, { + // `compiler.context/node_modules/react-native` may not exist, e.g. when + // react-native is hoisted to the monorepo root. + resolveReactNativeFrom: this.config.reactNativePath + ? undefined + : compiler.context, + }); + return resolvedHermesCLIPath; + }; const emittedAssets = new Map(); @@ -136,6 +147,12 @@ export class HermesBytecodePlugin { const assets = Array.from(emittedAssets); emittedAssets.clear(); + if (assets.length === 0) { + return; + } + + const hermesCLIPath = resolveHermesCLIPath(); + await Promise.all( assets.map(async ([file, outputPath]) => { const bundlePath = path.join(outputPath, file); diff --git a/packages/repack/src/plugins/HermesBytecodePlugin/utils/getHermesCLIPath.ts b/packages/repack/src/plugins/HermesBytecodePlugin/utils/getHermesCLIPath.ts index 61b031414..fc5625fb8 100644 --- a/packages/repack/src/plugins/HermesBytecodePlugin/utils/getHermesCLIPath.ts +++ b/packages/repack/src/plugins/HermesBytecodePlugin/utils/getHermesCLIPath.ts @@ -18,12 +18,113 @@ const getHermesOSBin = (): string | null => { } }; +/** + * Whether the binary exists. On Windows the file is `hermesc.exe`, which `execa` + * resolves from the extensionless path, so both spellings count. + */ +const binaryExists = (binaryPath: string): boolean => + fs.existsSync(binaryPath) || + (os.platform() === 'win32' && fs.existsSync(`${binaryPath}.exe`)); + +const isDirectory = (dirPath: string): boolean => { + try { + return fs.statSync(dirPath).isDirectory(); + } catch { + return false; + } +}; + +const tryRealpath = (filePath: string): string | undefined => { + try { + return fs.realpathSync(filePath); + } catch { + return undefined; + } +}; + +/** + * Finds `packageName` the way Node resolves it from `fromDir`: in `node_modules` of + * `fromDir` and each of its ancestors. Unlike `require.resolve`, `NODE_PATH` and global + * folders are not consulted: package manager shims (e.g. pnpm's) set `NODE_PATH`, which + * could yield an unrelated copy, and a mismatched hermesc emits bytecode the app's + * Hermes runtime can't load. + */ +const resolvePackageDir = ( + packageName: string, + fromDir: string +): string | undefined => { + let dir = path.resolve(fromDir); + + while (true) { + if (path.basename(dir) !== 'node_modules') { + const packageDir = path.join(dir, 'node_modules', packageName); + if (fs.existsSync(path.join(packageDir, 'package.json'))) { + return tryRealpath(packageDir) ?? packageDir; + } + } + + const parentDir = path.dirname(dir); + if (parentDir === dir) { + return undefined; + } + dir = parentDir; + } +}; + +/** + * Locations where `hermesc` may live for a given `react-native` package, + * beyond the lexical `/../hermes-compiler` join: + * - next to the real (symlink-resolved) `react-native`, e.g. pnpm's `.pnpm//node_modules` + * - wherever `hermes-compiler` resolves from `react-native`, e.g. nested in + * `react-native/node_modules` when versions conflict, or hoisted further up. + */ +const getResolvedHermesCompilerDirs = (reactNativePath: string): string[] => { + const dirs: string[] = []; + const realReactNativePath = tryRealpath(reactNativePath); + + if (realReactNativePath) { + dirs.push(path.join(realReactNativePath, '..', 'hermes-compiler')); + } + + for (const fromDir of new Set([ + realReactNativePath ?? reactNativePath, + reactNativePath, + ])) { + const resolvedDir = resolvePackageDir('hermes-compiler', fromDir); + if (resolvedDir) { + dirs.push(resolvedDir); + } + } + + return dirs; +}; + +interface GetHermesCLIPathOptions { + /** + * Directory to resolve `react-native` from when `reactNativePath` does not exist, + * e.g. a workspace whose `react-native` is hoisted to the monorepo root. + * Used only to locate `hermesc`. + */ + resolveReactNativeFrom?: string; +} + /** * Determines the path to the Hermes compiler binary. * - * Defaults to './node_modules/hermes-compiler/hermesc/{os-bin}/hermesc' + * Checks, in order: + * 1. `/../hermes-compiler/hermesc/{os-bin}/hermesc` (RN >= 0.82, hoisted) + * 2. `/sdks/hermesc/{os-bin}/hermesc` (RN < 0.82) + * 3. `hermes-compiler` next to the symlink-resolved `react-native` (e.g. pnpm), + * then wherever `hermes-compiler` resolves from `react-native` + * 4. The same locations for `react-native` resolved from `resolveReactNativeFrom`, + * when `reactNativePath` is not a directory. + * + * @throws When no Hermes compiler binary is found, listing every path checked. */ -export const getHermesCLIPath = (reactNativePath: string): string => { +export const getHermesCLIPath = ( + reactNativePath: string, + { resolveReactNativeFrom }: GetHermesCLIPathOptions = {} +): string => { const osBin = getHermesOSBin(); if (!osBin) { @@ -33,19 +134,69 @@ export const getHermesCLIPath = (reactNativePath: string): string => { ); } - const hermesCompilerPath = path.join( + const getBinaryPath = (hermesCompilerDir: string) => + path.join(hermesCompilerDir, 'hermesc', osBin, 'hermesc'); + + const hermesCompilerPath = getBinaryPath( + path.join(reactNativePath, '..', 'hermes-compiler') + ); + + // Kept exactly as before so that existing setups resolve to the same binary. + if (fs.existsSync(hermesCompilerPath)) { + return hermesCompilerPath; + } + + // Previous hermesc path in older react native versions, <0.82. + const legacyHermesCLIPath = path.join( reactNativePath, - '..', - 'hermes-compiler', + 'sdks', 'hermesc', osBin, 'hermesc' ); - if (fs.existsSync(hermesCompilerPath)) { - return hermesCompilerPath; + if (binaryExists(legacyHermesCLIPath)) { + return legacyHermesCLIPath; } - // Fallback to the previous hermesc path in older react native versions, <0.82. - return path.join(reactNativePath, 'sdks', 'hermesc', osBin, 'hermesc'); + const candidates = [ + // Only differs from the check above on Windows, where the binary is `hermesc.exe`. + hermesCompilerPath, + ...getResolvedHermesCompilerDirs(reactNativePath).map(getBinaryPath), + ]; + + if (resolveReactNativeFrom && !isDirectory(reactNativePath)) { + const resolvedReactNativePath = resolvePackageDir( + 'react-native', + resolveReactNativeFrom + ); + + if (resolvedReactNativePath) { + candidates.push( + getBinaryPath( + path.join(resolvedReactNativePath, '..', 'hermes-compiler') + ), + path.join(resolvedReactNativePath, 'sdks', 'hermesc', osBin, 'hermesc'), + ...getResolvedHermesCompilerDirs(resolvedReactNativePath).map( + getBinaryPath + ) + ); + } + } + + const found = candidates.find(binaryExists); + if (found) { + return found; + } + + const checkedPaths = [ + ...new Set([hermesCompilerPath, legacyHermesCLIPath, ...candidates]), + ]; + + throw new Error( + '[RepackHermesBytecodePlugin] Could not find the Hermes compiler (hermesc). ' + + `Checked:\n${checkedPaths.map((p) => ` - ${p}`).join('\n')}\n` + + 'Set the `hermesCLIPath` option to the hermesc binary, ' + + 'or `reactNativePath` to the react-native package that ships it.' + ); }; diff --git a/packages/repack/src/plugins/__tests__/HermesBytecodePlugin.test.ts b/packages/repack/src/plugins/__tests__/HermesBytecodePlugin.test.ts index d4625da68..e7f45ea3a 100644 --- a/packages/repack/src/plugins/__tests__/HermesBytecodePlugin.test.ts +++ b/packages/repack/src/plugins/__tests__/HermesBytecodePlugin.test.ts @@ -4,12 +4,13 @@ import path from 'node:path'; import { type Compiler, ModuleFilenameHelpers } from '@rspack/core'; import execa from 'execa'; import { HermesBytecodePlugin } from '../HermesBytecodePlugin/index.js'; -import { getHermesCLIPath } from '../HermesBytecodePlugin/utils/getHermesCLIPath.js'; jest.mock('node:fs', () => ({ __esModule: true, default: { existsSync: jest.fn(), + realpathSync: jest.fn(), + statSync: jest.fn(), promises: { access: jest.fn(), rename: jest.fn(), @@ -213,29 +214,96 @@ describe('HermesBytecodePlugin', () => { ]); }); - describe('getHermesCLIPath', () => { - const reactNativePath = 'path/to/react-native'; + describe('hermesc lookup', () => { + const reactNativePath = path.resolve('/project/node_modules/react-native'); + const config = { + enabled: true, + test: /\.(js)?bundle$/, + exclude: /index.bundle$/, + reactNativePath, + }; - it('returns new hermes-compiler path when it exists', () => { + beforeEach(() => { jest.spyOn(os, 'platform').mockReturnValue('darwin'); - jest.mocked(fs.existsSync).mockReturnValue(true); + const fsMock = fs.promises as jest.Mocked; + fsMock.access.mockRejectedValue(new Error('File not found')); + }); - const hermesPath = getHermesCLIPath(reactNativePath); + it('uses hermesCLIPath without looking up hermesc', async () => { + const pluginInstance = new HermesBytecodePlugin({ + ...config, + hermesCLIPath: 'path/to/hermesc', + }); - expect(hermesPath).toBe( - path.normalize('path/to/hermes-compiler/hermesc/osx-bin/hermesc') - ); + pluginInstance.apply(compilerMock as unknown as Compiler); + const [, afterEmit] = + compilerMock.hooks.afterEmit.tapPromise.mock.calls[0]; + + emitAssets(['a.chunk.bundle']); + await afterEmit(); + + expect(fs.existsSync).not.toHaveBeenCalled(); + expect(jest.mocked(execa).mock.calls[0][0]).toBe('path/to/hermesc'); }); - it('falls back to legacy hermesc path when hermes-compiler does not exist', () => { - jest.spyOn(os, 'platform').mockReturnValue('darwin'); - jest.mocked(fs.existsSync).mockReturnValue(false); + it('does not look up hermesc until a matching asset is emitted', async () => { + const pluginInstance = new HermesBytecodePlugin(config); - const hermesPath = getHermesCLIPath(reactNativePath); + expect(() => + pluginInstance.apply(compilerMock as unknown as Compiler) + ).not.toThrow(); + const [, afterEmit] = + compilerMock.hooks.afterEmit.tapPromise.mock.calls[0]; - expect(hermesPath).toBe( - path.normalize('path/to/react-native/sdks/hermesc/osx-bin/hermesc') + emitAssets(['index.bundle', 'image.png']); + await expect(afterEmit()).resolves.toBeUndefined(); + + expect(fs.existsSync).not.toHaveBeenCalled(); + expect(execa).not.toHaveBeenCalled(); + }); + + it('fails the emit with the checked paths when hermesc is not found', async () => { + const pluginInstance = new HermesBytecodePlugin(config); + + pluginInstance.apply(compilerMock as unknown as Compiler); + const [, afterEmit] = + compilerMock.hooks.afterEmit.tapPromise.mock.calls[0]; + + emitAssets(['a.chunk.bundle']); + const error = await afterEmit().catch((e: Error) => e); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain('Could not find the Hermes compiler'); + expect(error.message).toContain( + path.join(reactNativePath, 'sdks/hermesc/osx-bin/hermesc') + ); + expect(execa).not.toHaveBeenCalled(); + }); + + it('looks up hermesc once and reuses it', async () => { + jest.mocked(fs.existsSync).mockReturnValue(true); + const pluginInstance = new HermesBytecodePlugin(config); + + pluginInstance.apply(compilerMock as unknown as Compiler); + const [, afterEmit] = + compilerMock.hooks.afterEmit.tapPromise.mock.calls[0]; + + emitAssets(['a.chunk.bundle', 'b.chunk.bundle']); + await afterEmit(); + emitAssets(['c.chunk.bundle']); + await afterEmit(); + + const hermesCompilerPath = path.join( + reactNativePath, + '..', + 'hermes-compiler/hermesc/osx-bin/hermesc' ); + expect(fs.existsSync).toHaveBeenCalledTimes(1); + expect(jest.mocked(execa).mock.calls.map(([file]) => file)).toEqual([ + hermesCompilerPath, + hermesCompilerPath, + hermesCompilerPath, + ]); }); }); }); diff --git a/packages/repack/src/plugins/__tests__/getHermesCLIPath.test.ts b/packages/repack/src/plugins/__tests__/getHermesCLIPath.test.ts new file mode 100644 index 000000000..41deab118 --- /dev/null +++ b/packages/repack/src/plugins/__tests__/getHermesCLIPath.test.ts @@ -0,0 +1,205 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { getHermesCLIPath } from '../HermesBytecodePlugin/utils/getHermesCLIPath.js'; + +function createFile(filePath: string, content = '') { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); +} + +function createPackage(packagePath: string, name: string) { + createFile( + path.join(packagePath, 'package.json'), + JSON.stringify({ name, version: '0.0.0' }) + ); +} + +/** Creates `hermes-compiler` with its binary and returns the binary path. */ +function createHermesCompiler(packagePath: string, binary = 'hermesc') { + createPackage(packagePath, 'hermes-compiler'); + const binaryPath = path.join(packagePath, 'hermesc', 'osx-bin', binary); + createFile(binaryPath); + return binaryPath; +} + +function symlinkDir(target: string, linkPath: string) { + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + // junctions don't need elevated permissions on Windows; ignored elsewhere + fs.symlinkSync(target, linkPath, 'junction'); +} + +describe('getHermesCLIPath', () => { + let root: string; + + beforeEach(() => { + root = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'repack-hermesc-')) + ); + jest.spyOn(os, 'platform').mockReturnValue('darwin'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('returns hermes-compiler next to react-native (hoisted layout)', () => { + const reactNativePath = path.join(root, 'node_modules', 'react-native'); + createPackage(reactNativePath, 'react-native'); + createHermesCompiler(path.join(root, 'node_modules', 'hermes-compiler')); + // a legacy binary must not take precedence + createFile(path.join(reactNativePath, 'sdks/hermesc/osx-bin/hermesc')); + + expect(getHermesCLIPath(reactNativePath)).toBe( + path.join( + reactNativePath, + '..', + 'hermes-compiler/hermesc/osx-bin/hermesc' + ) + ); + }); + + it('returns legacy sdks/hermesc path for react-native < 0.82', () => { + const reactNativePath = path.join(root, 'node_modules', 'react-native'); + createPackage(reactNativePath, 'react-native'); + const legacyPath = path.join( + reactNativePath, + 'sdks/hermesc/osx-bin/hermesc' + ); + createFile(legacyPath); + // resolvable from react-native, but the legacy path is checked first + createHermesCompiler( + path.join(reactNativePath, 'node_modules', 'hermes-compiler') + ); + + expect(getHermesCLIPath(reactNativePath)).toBe(legacyPath); + }); + + it('follows a symlinked react-native to hermes-compiler (pnpm layout)', () => { + const storePath = path.join( + root, + 'node_modules/.pnpm/react-native@0.88.1/node_modules' + ); + const realReactNativePath = path.join(storePath, 'react-native'); + createPackage(realReactNativePath, 'react-native'); + const binaryPath = createHermesCompiler( + path.join(storePath, 'hermes-compiler') + ); + + const reactNativePath = path.join( + root, + 'packages/app/node_modules/react-native' + ); + symlinkDir(realReactNativePath, reactNativePath); + + expect(getHermesCLIPath(reactNativePath)).toBe(binaryPath); + }); + + it('resolves hermes-compiler from react-native (nested layout)', () => { + const reactNativePath = path.join(root, 'node_modules', 'react-native'); + createPackage(reactNativePath, 'react-native'); + const binaryPath = createHermesCompiler( + path.join(reactNativePath, 'node_modules', 'hermes-compiler') + ); + + expect(getHermesCLIPath(reactNativePath)).toBe(binaryPath); + }); + + it('resolves react-native from resolveReactNativeFrom when reactNativePath does not exist', () => { + const appPath = path.join(root, 'packages', 'app'); + fs.mkdirSync(appPath, { recursive: true }); + const hoistedReactNativePath = path.join( + root, + 'node_modules', + 'react-native' + ); + createPackage(hoistedReactNativePath, 'react-native'); + // nested, so it's only reachable through the hoisted react-native + const binaryPath = createHermesCompiler( + path.join(hoistedReactNativePath, 'node_modules', 'hermes-compiler') + ); + const reactNativePath = path.join(appPath, 'node_modules', 'react-native'); + + expect(() => getHermesCLIPath(reactNativePath)).toThrow( + 'Could not find the Hermes compiler' + ); + expect( + getHermesCLIPath(reactNativePath, { resolveReactNativeFrom: appPath }) + ).toBe(binaryPath); + }); + + it('does not use resolveReactNativeFrom when reactNativePath exists', () => { + const appPath = path.join(root, 'packages', 'app'); + const reactNativePath = path.join(appPath, 'node_modules', 'react-native'); + createPackage(reactNativePath, 'react-native'); + const hoistedReactNativePath = path.join( + root, + 'node_modules', + 'react-native' + ); + createPackage(hoistedReactNativePath, 'react-native'); + createHermesCompiler( + path.join(hoistedReactNativePath, 'node_modules', 'hermes-compiler') + ); + + expect(() => + getHermesCLIPath(reactNativePath, { resolveReactNativeFrom: appPath }) + ).toThrow('Could not find the Hermes compiler'); + }); + + it('throws an error listing every checked path when hermesc is not found', () => { + const reactNativePath = path.join(root, 'node_modules', 'react-native'); + createPackage(reactNativePath, 'react-native'); + + let error: Error | undefined; + try { + getHermesCLIPath(reactNativePath); + } catch (e) { + error = e as Error; + } + + expect(error?.message).toContain( + path.join(root, 'node_modules/hermes-compiler/hermesc/osx-bin/hermesc') + ); + expect(error?.message).toContain( + path.join(reactNativePath, 'sdks/hermesc/osx-bin/hermesc') + ); + expect(error?.message).toContain('hermesCLIPath'); + }); + + it('throws when the OS is not recognized', () => { + jest.spyOn(os, 'platform').mockReturnValue('aix'); + + expect(() => getHermesCLIPath(root)).toThrow('OS not recognized'); + }); + + describe('on Windows', () => { + beforeEach(() => { + jest.spyOn(os, 'platform').mockReturnValue('win32'); + }); + + it('returns legacy path without the .exe suffix', () => { + const reactNativePath = path.join(root, 'node_modules', 'react-native'); + const legacyPath = path.join( + reactNativePath, + 'sdks/hermesc/win64-bin/hermesc' + ); + createFile(`${legacyPath}.exe`); + + expect(getHermesCLIPath(reactNativePath)).toBe(legacyPath); + }); + + it('finds hermesc.exe in hermes-compiler', () => { + const reactNativePath = path.join(root, 'node_modules', 'react-native'); + createPackage(reactNativePath, 'react-native'); + const binaryPath = path.join( + root, + 'node_modules/hermes-compiler/hermesc/win64-bin/hermesc' + ); + createFile(`${binaryPath}.exe`); + + expect(getHermesCLIPath(reactNativePath)).toBe(binaryPath); + }); + }); +}); diff --git a/website/src/latest/api/plugins/hermes-bytecode.md b/website/src/latest/api/plugins/hermes-bytecode.md index ff786e5c7..e7b1e3d07 100644 --- a/website/src/latest/api/plugins/hermes-bytecode.md +++ b/website/src/latest/api/plugins/hermes-bytecode.md @@ -67,7 +67,13 @@ Excludes all modules that match this condition and will match against the absolu - Type: `string` - Default: `undefined` -Path to the Hermes command-line tool. If not specified, the plugin will try to find it automatically. +Path to the Hermes command-line tool. If not specified, the plugin looks for the `hermesc` binary shipped with React Native when the first matching asset is transformed: + +1. `hermes-compiler` next to `react-native` (React Native 0.82 and newer). +2. `react-native/sdks/hermesc` (React Native older than 0.82). +3. `hermes-compiler` next to the symlink-resolved `react-native` (e.g. pnpm), or resolved from `react-native`'s `node_modules` hierarchy. + +If `reactNativePath` is not set and `/node_modules/react-native` doesn't exist, `react-native` is also resolved from the project context for this lookup. If no binary is found, the build fails with an error listing every path that was checked. ### reactNativePath From 47e35c799304a95f176952f53f41c25f91dcb33d Mon Sep 17 00:00:00 2001 From: Daniel Williams Date: Wed, 4 Nov 2026 13:47:33 +0000 Subject: [PATCH 2/2] fix: exit non-zero when the bundle compiler fails `bundle` awaited its async error handler inside the compiler's `close` callback, so a compiler error (e.g. a failed hermesc run in HermesBytecodePlugin) became an unhandled rejection and the command promise never settled. LoggerPlugin's `uncaughtException` listener then logged it and, because registering the listener disables Node's default exit code, the process exited 0 with un-compiled chunks on disk. `bundle` now rejects with the handler's error in run and watch mode, and LoggerPlugin sets `process.exitCode = 1` for uncaught exceptions. --- .changeset/bundle-fail-on-compiler-error.md | 5 + .../src/commands/__tests__/bundle.test.ts | 57 +++++++++++ packages/repack/src/commands/bundle.ts | 95 ++++++++++++------- packages/repack/src/plugins/LoggerPlugin.ts | 3 + .../plugins/__tests__/LoggerPlugin.test.ts | 49 ++++++++++ 5 files changed, 174 insertions(+), 35 deletions(-) create mode 100644 .changeset/bundle-fail-on-compiler-error.md create mode 100644 packages/repack/src/commands/__tests__/bundle.test.ts create mode 100644 packages/repack/src/plugins/__tests__/LoggerPlugin.test.ts diff --git a/.changeset/bundle-fail-on-compiler-error.md b/.changeset/bundle-fail-on-compiler-error.md new file mode 100644 index 000000000..57cd575a7 --- /dev/null +++ b/.changeset/bundle-fail-on-compiler-error.md @@ -0,0 +1,5 @@ +--- +"@callstack/repack": patch +--- + +A failed Hermes bytecode compilation now fails the build instead of exiting 0. `bundle` rejected inside the compiler's `close` callback, so compiler errors (such as a failing hermesc in `HermesBytecodePlugin`) became unhandled rejections; `LoggerPlugin`'s `uncaughtException` listener then logged them and the process exited 0, leaving plain JavaScript chunks in place. `bundle` now rejects (in both run and `--watch` mode) and `LoggerPlugin` sets `process.exitCode = 1` for uncaught exceptions. diff --git a/packages/repack/src/commands/__tests__/bundle.test.ts b/packages/repack/src/commands/__tests__/bundle.test.ts new file mode 100644 index 000000000..73556b686 --- /dev/null +++ b/packages/repack/src/commands/__tests__/bundle.test.ts @@ -0,0 +1,57 @@ +import { type BundleCompiler, runCompiler } from '../bundle.js'; + +type Callback = (error: Error | null) => void; + +function createCompilerMock(error: Error | null) { + return { + run: jest.fn((callback: Callback) => callback(error)), + watch: jest.fn((_: unknown, callback: Callback) => callback(error)), + close: jest.fn((callback: Callback) => callback(null)), + hooks: { watchClose: { tap: jest.fn() } }, + options: { stats: undefined }, + context: '/project', + } satisfies BundleCompiler; +} + +// mirrors `bundle`'s handler, which throws a CLIError for compiler errors +const onResult = async (error: Error | null) => { + if (error) throw error; +}; + +describe('runCompiler', () => { + it('resolves after closing the compiler when the build succeeds', async () => { + const compiler = createCompilerMock(null); + + await expect(runCompiler(compiler, { onResult })).resolves.toBeUndefined(); + expect(compiler.close).toHaveBeenCalledTimes(1); + }); + + it('rejects when the build fails', async () => { + const compiler = createCompilerMock(new Error('hermesc failed')); + + await expect(runCompiler(compiler, { onResult })).rejects.toThrow( + 'hermesc failed' + ); + expect(compiler.close).toHaveBeenCalledTimes(1); + }); + + it('rejects when a build fails in watch mode', async () => { + const compiler = createCompilerMock(new Error('hermesc failed')); + + await expect( + runCompiler(compiler, { watch: true, onResult }) + ).rejects.toThrow('hermesc failed'); + expect(compiler.run).not.toHaveBeenCalled(); + }); + + it('resolves when watching stops', async () => { + const compiler = createCompilerMock(null); + compiler.hooks.watchClose.tap.mockImplementation( + (_: string, onClose: () => void) => onClose() + ); + + await expect( + runCompiler(compiler, { watch: true, onResult }) + ).resolves.toBeUndefined(); + }); +}); diff --git a/packages/repack/src/commands/bundle.ts b/packages/repack/src/commands/bundle.ts index 894da4639..7d4ccc9dd 100644 --- a/packages/repack/src/commands/bundle.ts +++ b/packages/repack/src/commands/bundle.ts @@ -21,7 +21,7 @@ import type { * Both rspack() and webpack() return objects satisfying this shape. * Defined here because both bundlers are optional peer dependencies. */ -interface BundleCompiler { +export interface BundleCompiler { run(callback: (error: Error | null, stats?: BundleStats) => void): void; watch( options: Record, @@ -37,7 +37,7 @@ interface BundleCompiler { * Minimal stats interface for the bundle command. * Both rspack and webpack Stats objects satisfy this shape. */ -interface BundleStats { +export interface BundleStats { hasErrors(): boolean; compilation?: { errors?: unknown[] }; toJson(options: unknown): Record; @@ -118,48 +118,73 @@ export async function bundle( compiler = webpack(config) as BundleCompiler; } - return new Promise((resolve) => { - const errorHandler = async (error: Error | null, stats?: BundleStats) => { - if (error) { - throw new CLIError(error.message); - } + const errorHandler = async (error: Error | null, stats?: BundleStats) => { + if (error) { + throw new CLIError(error.message); + } + + if (stats?.hasErrors()) { + stats.compilation?.errors?.forEach((e) => { + console.error(e); + }); + process.exit(2); + } + + if (args.json && stats !== undefined) { + const statsOptions = normalizeStatsOptions( + compiler.options.stats, + args.stats + ); - if (stats?.hasErrors()) { - stats.compilation?.errors?.forEach((e) => { - console.error(e); + const statsJson = stats.toJson(statsOptions); + + try { + await writeStats(statsJson, { + filepath: args.json, + rootDir: compiler.context, }); - process.exit(2); + } catch (e) { + throw new CLIError(String(e)); } + } + }; - if (args.json && stats !== undefined) { - const statsOptions = normalizeStatsOptions( - compiler.options.stats, - args.stats - ); - - const statsJson = stats.toJson(statsOptions); - - try { - await writeStats(statsJson, { - filepath: args.json, - rootDir: compiler.context, - }); - } catch (e) { - throw new CLIError(String(e)); - } - } - }; + return runCompiler(compiler, { + watch: args.watch, + watchOptions: config.watchOptions, + onResult: errorHandler, + }); +} - if (args.watch) { +/** + * Runs (or watches) the compiler. The returned promise settles with `onResult` + * of the run, so a failed build rejects and the CLI exits with a non-zero code. + * In watch mode it resolves when watching stops, or rejects on the first failure. + */ +export function runCompiler( + compiler: BundleCompiler, + { + watch, + watchOptions, + onResult, + }: { + watch?: boolean; + watchOptions?: Record; + onResult: (error: Error | null, stats?: BundleStats) => Promise; + } +) { + return new Promise((resolve, reject) => { + if (watch) { compiler.hooks.watchClose.tap('bundle', resolve); - compiler.watch(config.watchOptions ?? {}, errorHandler); + compiler.watch(watchOptions ?? {}, (error, stats) => { + onResult(error, stats).catch(reject); + }); } else { - compiler.run((error: Error | null, stats?: BundleStats) => { + compiler.run((error, stats) => { // make cache work: https://webpack.js.org/api/node/#run - compiler.close(async (closeErr: Error | null) => { + compiler.close((closeErr) => { if (closeErr) console.error(closeErr); - await errorHandler(error, stats); - resolve(); + onResult(error, stats).then(resolve, reject); }); }); } diff --git a/packages/repack/src/plugins/LoggerPlugin.ts b/packages/repack/src/plugins/LoggerPlugin.ts index c72a9ab37..ff209a9e3 100644 --- a/packages/repack/src/plugins/LoggerPlugin.ts +++ b/packages/repack/src/plugins/LoggerPlugin.ts @@ -202,6 +202,9 @@ export class LoggerPlugin { } this.reporter.flush(); this.reporter.stop(); + // Registering this listener disables Node's default exit code 1 for uncaught errors. + // Set it explicitly instead of calling `process.exit` so pending output is flushed. + process.exitCode = 1; }); process.on('exit', () => { diff --git a/packages/repack/src/plugins/__tests__/LoggerPlugin.test.ts b/packages/repack/src/plugins/__tests__/LoggerPlugin.test.ts new file mode 100644 index 000000000..75b27e952 --- /dev/null +++ b/packages/repack/src/plugins/__tests__/LoggerPlugin.test.ts @@ -0,0 +1,49 @@ +import type { Compiler } from '@rspack/core'; +import { LoggerPlugin } from '../LoggerPlugin.js'; + +const compilerMock = { + options: {}, + hooks: { + infrastructureLog: { tap: jest.fn() }, + thisCompilation: { tap: jest.fn() }, + afterDone: { tap: jest.fn() }, + }, +}; + +describe('LoggerPlugin', () => { + let exceptionListeners: NodeJS.UncaughtExceptionListener[]; + let exitListeners: NodeJS.ExitListener[]; + + beforeEach(() => { + exceptionListeners = process.listeners('uncaughtException'); + exitListeners = process.listeners('exit'); + }); + + afterEach(() => { + for (const listener of process.listeners('uncaughtException')) { + if (!exceptionListeners.includes(listener)) { + process.off('uncaughtException', listener); + } + } + for (const listener of process.listeners('exit')) { + if (!exitListeners.includes(listener)) { + process.off('exit', listener); + } + } + process.exitCode = undefined; + }); + + it('sets a failing exit code on uncaught exceptions', () => { + const plugin = new LoggerPlugin({ output: {} }); + plugin.apply(compilerMock as unknown as Compiler); + + const listener = process + .listeners('uncaughtException') + .find((l) => !exceptionListeners.includes(l)); + + expect(listener).toBeDefined(); + listener?.(new Error('boom'), 'uncaughtException'); + + expect(process.exitCode).toBe(1); + }); +});