From 422460599b63db0df617898fe7130c84cd361993 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tr=E1=BA=A7n=20=C4=90=C3=ACnh=20Huy?= Date: Sun, 23 Aug 2026 13:27:22 +0700 Subject: [PATCH] feat(host): add fail-on-error link option --- .changeset/strict-link-resolution.md | 8 +++ docs/CLI.md | 1 + packages/host/src/node/cli/bin.test.ts | 70 ++++++++++++++++++++++ packages/host/src/node/cli/link-modules.ts | 5 +- packages/host/src/node/cli/program.ts | 10 +++- packages/host/src/node/path-utils.ts | 27 +++++++-- 6 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 .changeset/strict-link-resolution.md diff --git a/.changeset/strict-link-resolution.md b/.changeset/strict-link-resolution.md new file mode 100644 index 00000000..5ebc74a2 --- /dev/null +++ b/.changeset/strict-link-resolution.md @@ -0,0 +1,8 @@ +--- +"react-native-node-api": minor +--- + +Add a `--fail-on-error` option to `react-native-node-api link`. By default, +unresolvable dependencies continue to be skipped with a warning; the new flag +instead surfaces the original package-resolution error and exits unsuccessfully, +which makes broken package `exports` configurations diagnosable in CI. diff --git a/docs/CLI.md b/docs/CLI.md index fab4b1a5..5213c8b7 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -56,6 +56,7 @@ Auto-links the Node-API modules found among the app's dependencies for one or mo - `--android` — Link Android modules. - `--apple` — Link Apple modules. - `--prune` — Delete previously vendored modules that are no longer auto-linked. Defaults to `true`. +- `--fail-on-error` — Fail with the original package-resolution error instead of skipping dependencies that cannot be resolved. Defaults to `false`. - `--package-name ` — Controls how a dependency's package name is transformed into a library name. One of `strip`, `keep` or `omit` (see [Library naming](#library-naming) below). Defaults to `strip`, or the `NODE_API_PACKAGE_NAME` environment variable if set. - `--path-suffix ` — Controls how the path of the addon inside a package is transformed into a library name. One of `strip`, `keep` or `omit` (see [Library naming](#library-naming) below). Defaults to `strip`, or the `NODE_API_PATH_SUFFIX` environment variable if set. diff --git a/packages/host/src/node/cli/bin.test.ts b/packages/host/src/node/cli/bin.test.ts index 5adc185c..0b4190f6 100644 --- a/packages/host/src/node/cli/bin.test.ts +++ b/packages/host/src/node/cli/bin.test.ts @@ -69,5 +69,75 @@ describe("bin", () => { `Failed to find expected output (stdout: ${stdout} stderr: ${stderr})`, ); }); + + it("skips dependencies that cannot be resolved by default", (context) => { + const targetBuildDir = setupTempDirectory(context, {}); + const appDir = setupTempDirectory(context, { + "package.json": JSON.stringify({ + name: "test-app", + dependencies: { "broken-package": "1.0.0" }, + }), + "node_modules/broken-package/package.json": JSON.stringify({ + name: "broken-package", + exports: "./missing.js", + }), + }); + + const { status, stdout, stderr } = cp.spawnSync( + process.execPath, + [BIN_PATH, "link", appDir, "--android"], + { + cwd: PACKAGE_ROOT, + encoding: "utf8", + env: { + ...process.env, + TARGET_BUILD_DIR: targetBuildDir, + }, + }, + ); + + assert.equal( + status, + 0, + `Expected success (got ${status}): ${stdout} ${stderr}`, + ); + assert.match(stderr, /Cannot find package root .* for broken-package/); + }); + + it("reports dependency resolution errors with --fail-on-error", (context) => { + const targetBuildDir = setupTempDirectory(context, {}); + const appDir = setupTempDirectory(context, { + "package.json": JSON.stringify({ + name: "test-app", + dependencies: { "broken-package": "1.0.0" }, + }), + "node_modules/broken-package/package.json": JSON.stringify({ + name: "broken-package", + exports: "./missing.js", + }), + }); + + const { status, stdout, stderr } = cp.spawnSync( + process.execPath, + [BIN_PATH, "link", appDir, "--android", "--fail-on-error"], + { + cwd: PACKAGE_ROOT, + encoding: "utf8", + env: { + ...process.env, + TARGET_BUILD_DIR: targetBuildDir, + }, + }, + ); + + assert.equal( + status, + 1, + `Expected failure (got ${status}): ${stdout} ${stderr}`, + ); + assert.match(stderr, /broken-package/); + assert.match(stderr, /missing\.js/); + assert.doesNotMatch(stderr, /unknown option/); + }); }); }); diff --git a/packages/host/src/node/cli/link-modules.ts b/packages/host/src/node/cli/link-modules.ts index 2c2855a0..22f349c4 100644 --- a/packages/host/src/node/cli/link-modules.ts +++ b/packages/host/src/node/cli/link-modules.ts @@ -26,11 +26,12 @@ export type LinkModulesOptions = { naming: NamingStrategy; fromPath: string; linker: ModuleLinker; + failOnError?: boolean; }; export type LinkModuleOptions = Omit< LinkModulesOptions, - "fromPath" | "linker" | "platform" + "fromPath" | "linker" | "platform" | "failOnError" > & { modulePath: string; }; @@ -63,12 +64,14 @@ export async function linkModules({ naming, platform, linker, + failOnError, }: LinkModulesOptions): Promise { // Find all their xcframeworks const dependenciesByName = await findNodeApiModulePathsByDependency({ fromPath, platform, includeSelf: true, + failOnError, }); // Find absolute paths to xcframeworks diff --git a/packages/host/src/node/cli/program.ts b/packages/host/src/node/cli/program.ts index 9078d26f..2f91efae 100644 --- a/packages/host/src/node/cli/program.ts +++ b/packages/host/src/node/cli/program.ts @@ -62,11 +62,18 @@ program ) .option("--android", "Link Android modules") .option("--apple", "Link Apple modules") + .option( + "--fail-on-error", + "Fail when a package dependency cannot be resolved", + ) .addOption(packageNameOption) .addOption(pathSuffixOption) .action( wrapAction( - async (pathArg, { prune, pathSuffix, android, apple, packageName }) => { + async ( + pathArg, + { prune, pathSuffix, android, apple, packageName, failOnError }, + ) => { console.log("Auto-linking Node-API modules from", chalk.dim(pathArg)); const platforms: PlatformName[] = []; if (android) { @@ -94,6 +101,7 @@ program fromPath: path.resolve(pathArg), naming: { packageName, pathSuffix }, linker: await createLinker(platform), + failOnError, }), { text: `Linking ${platformDisplayName} Node-API modules`, diff --git a/packages/host/src/node/path-utils.ts b/packages/host/src/node/path-utils.ts index 0cb4506a..10dce656 100644 --- a/packages/host/src/node/path-utils.ts +++ b/packages/host/src/node/path-utils.ts @@ -253,13 +253,20 @@ export function getLibraryName(modulePath: string, naming: NamingStrategy) { return parts.join("--"); } +function resolvePackageRootOrThrow( + requireFromPackageRoot: NodeJS.Require, + packageName: string, +): string | undefined { + const resolvedPath = requireFromPackageRoot.resolve(packageName); + return packageDirectorySync({ cwd: resolvedPath }); +} + export function resolvePackageRoot( requireFromPackageRoot: NodeJS.Require, packageName: string, ): string | undefined { try { - const resolvedPath = requireFromPackageRoot.resolve(packageName); - return packageDirectorySync({ cwd: resolvedPath }); + return resolvePackageRootOrThrow(requireFromPackageRoot, packageName); } catch { // TODO: Add a debug log here return undefined; @@ -356,6 +363,7 @@ export function findPackageConfigurationByPath( */ export function findPackageDependencyPaths( fromPath: string, + { failOnError = false }: { failOnError?: boolean } = {}, ): Record { const packageRoot = packageDirectorySync({ cwd: fromPath }); assert(packageRoot, `Could not find package root from ${fromPath}`); @@ -389,8 +397,15 @@ export function findPackageDependencyPaths( } visited.add(name); - const root = resolvePackageRoot(requireFromRoot, name); + const root = failOnError + ? resolvePackageRootOrThrow(requireFromRoot, name) + : resolvePackageRoot(requireFromRoot, name); if (!root) { + if (failOnError) { + throw new Error( + `Cannot find package root from ${fromPath} for ${name}`, + ); + } console.warn(`Cannot find package root from ${fromPath} for ${name}`); continue; } @@ -511,13 +526,17 @@ export async function findNodeApiModulePathsByDependency({ fromPath, includeSelf, excludePackages = DEFAULT_EXCLUDE_PACKAGES, + failOnError = false, ...options }: FindNodeApiModuleOptions & { includeSelf: boolean; excludePackages?: string[]; + failOnError?: boolean; }) { // Find the location of each dependency - const packagePathsByName = findPackageDependencyPaths(fromPath); + const packagePathsByName = findPackageDependencyPaths(fromPath, { + failOnError, + }); if (includeSelf) { const packageRoot = packageDirectorySync({ cwd: fromPath }); assert(packageRoot, `Could not find package root from ${fromPath}`);