Skip to content

Commit 4224605

Browse files
committed
feat(host): add fail-on-error link option
1 parent 0a29fbd commit 4224605

6 files changed

Lines changed: 115 additions & 6 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"react-native-node-api": minor
3+
---
4+
5+
Add a `--fail-on-error` option to `react-native-node-api link`. By default,
6+
unresolvable dependencies continue to be skipped with a warning; the new flag
7+
instead surfaces the original package-resolution error and exits unsuccessfully,
8+
which makes broken package `exports` configurations diagnosable in CI.

docs/CLI.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ Auto-links the Node-API modules found among the app's dependencies for one or mo
5656
- `--android` — Link Android modules.
5757
- `--apple` — Link Apple modules.
5858
- `--prune` — Delete previously vendored modules that are no longer auto-linked. Defaults to `true`.
59+
- `--fail-on-error` — Fail with the original package-resolution error instead of skipping dependencies that cannot be resolved. Defaults to `false`.
5960
- `--package-name <strategy>` — 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.
6061
- `--path-suffix <strategy>` — 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.
6162

packages/host/src/node/cli/bin.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,75 @@ describe("bin", () => {
6969
`Failed to find expected output (stdout: ${stdout} stderr: ${stderr})`,
7070
);
7171
});
72+
73+
it("skips dependencies that cannot be resolved by default", (context) => {
74+
const targetBuildDir = setupTempDirectory(context, {});
75+
const appDir = setupTempDirectory(context, {
76+
"package.json": JSON.stringify({
77+
name: "test-app",
78+
dependencies: { "broken-package": "1.0.0" },
79+
}),
80+
"node_modules/broken-package/package.json": JSON.stringify({
81+
name: "broken-package",
82+
exports: "./missing.js",
83+
}),
84+
});
85+
86+
const { status, stdout, stderr } = cp.spawnSync(
87+
process.execPath,
88+
[BIN_PATH, "link", appDir, "--android"],
89+
{
90+
cwd: PACKAGE_ROOT,
91+
encoding: "utf8",
92+
env: {
93+
...process.env,
94+
TARGET_BUILD_DIR: targetBuildDir,
95+
},
96+
},
97+
);
98+
99+
assert.equal(
100+
status,
101+
0,
102+
`Expected success (got ${status}): ${stdout} ${stderr}`,
103+
);
104+
assert.match(stderr, /Cannot find package root .* for broken-package/);
105+
});
106+
107+
it("reports dependency resolution errors with --fail-on-error", (context) => {
108+
const targetBuildDir = setupTempDirectory(context, {});
109+
const appDir = setupTempDirectory(context, {
110+
"package.json": JSON.stringify({
111+
name: "test-app",
112+
dependencies: { "broken-package": "1.0.0" },
113+
}),
114+
"node_modules/broken-package/package.json": JSON.stringify({
115+
name: "broken-package",
116+
exports: "./missing.js",
117+
}),
118+
});
119+
120+
const { status, stdout, stderr } = cp.spawnSync(
121+
process.execPath,
122+
[BIN_PATH, "link", appDir, "--android", "--fail-on-error"],
123+
{
124+
cwd: PACKAGE_ROOT,
125+
encoding: "utf8",
126+
env: {
127+
...process.env,
128+
TARGET_BUILD_DIR: targetBuildDir,
129+
},
130+
},
131+
);
132+
133+
assert.equal(
134+
status,
135+
1,
136+
`Expected failure (got ${status}): ${stdout} ${stderr}`,
137+
);
138+
assert.match(stderr, /broken-package/);
139+
assert.match(stderr, /missing\.js/);
140+
assert.doesNotMatch(stderr, /unknown option/);
141+
});
72142
});
73143
});

packages/host/src/node/cli/link-modules.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ export type LinkModulesOptions = {
2626
naming: NamingStrategy;
2727
fromPath: string;
2828
linker: ModuleLinker;
29+
failOnError?: boolean;
2930
};
3031

3132
export type LinkModuleOptions = Omit<
3233
LinkModulesOptions,
33-
"fromPath" | "linker" | "platform"
34+
"fromPath" | "linker" | "platform" | "failOnError"
3435
> & {
3536
modulePath: string;
3637
};
@@ -63,12 +64,14 @@ export async function linkModules({
6364
naming,
6465
platform,
6566
linker,
67+
failOnError,
6668
}: LinkModulesOptions): Promise<ModuleOutput[]> {
6769
// Find all their xcframeworks
6870
const dependenciesByName = await findNodeApiModulePathsByDependency({
6971
fromPath,
7072
platform,
7173
includeSelf: true,
74+
failOnError,
7275
});
7376

7477
// Find absolute paths to xcframeworks

packages/host/src/node/cli/program.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,18 @@ program
6262
)
6363
.option("--android", "Link Android modules")
6464
.option("--apple", "Link Apple modules")
65+
.option(
66+
"--fail-on-error",
67+
"Fail when a package dependency cannot be resolved",
68+
)
6569
.addOption(packageNameOption)
6670
.addOption(pathSuffixOption)
6771
.action(
6872
wrapAction(
69-
async (pathArg, { prune, pathSuffix, android, apple, packageName }) => {
73+
async (
74+
pathArg,
75+
{ prune, pathSuffix, android, apple, packageName, failOnError },
76+
) => {
7077
console.log("Auto-linking Node-API modules from", chalk.dim(pathArg));
7178
const platforms: PlatformName[] = [];
7279
if (android) {
@@ -94,6 +101,7 @@ program
94101
fromPath: path.resolve(pathArg),
95102
naming: { packageName, pathSuffix },
96103
linker: await createLinker(platform),
104+
failOnError,
97105
}),
98106
{
99107
text: `Linking ${platformDisplayName} Node-API modules`,

packages/host/src/node/path-utils.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,13 +253,20 @@ export function getLibraryName(modulePath: string, naming: NamingStrategy) {
253253
return parts.join("--");
254254
}
255255

256+
function resolvePackageRootOrThrow(
257+
requireFromPackageRoot: NodeJS.Require,
258+
packageName: string,
259+
): string | undefined {
260+
const resolvedPath = requireFromPackageRoot.resolve(packageName);
261+
return packageDirectorySync({ cwd: resolvedPath });
262+
}
263+
256264
export function resolvePackageRoot(
257265
requireFromPackageRoot: NodeJS.Require,
258266
packageName: string,
259267
): string | undefined {
260268
try {
261-
const resolvedPath = requireFromPackageRoot.resolve(packageName);
262-
return packageDirectorySync({ cwd: resolvedPath });
269+
return resolvePackageRootOrThrow(requireFromPackageRoot, packageName);
263270
} catch {
264271
// TODO: Add a debug log here
265272
return undefined;
@@ -356,6 +363,7 @@ export function findPackageConfigurationByPath(
356363
*/
357364
export function findPackageDependencyPaths(
358365
fromPath: string,
366+
{ failOnError = false }: { failOnError?: boolean } = {},
359367
): Record<string, string> {
360368
const packageRoot = packageDirectorySync({ cwd: fromPath });
361369
assert(packageRoot, `Could not find package root from ${fromPath}`);
@@ -389,8 +397,15 @@ export function findPackageDependencyPaths(
389397
}
390398
visited.add(name);
391399

392-
const root = resolvePackageRoot(requireFromRoot, name);
400+
const root = failOnError
401+
? resolvePackageRootOrThrow(requireFromRoot, name)
402+
: resolvePackageRoot(requireFromRoot, name);
393403
if (!root) {
404+
if (failOnError) {
405+
throw new Error(
406+
`Cannot find package root from ${fromPath} for ${name}`,
407+
);
408+
}
394409
console.warn(`Cannot find package root from ${fromPath} for ${name}`);
395410
continue;
396411
}
@@ -511,13 +526,17 @@ export async function findNodeApiModulePathsByDependency({
511526
fromPath,
512527
includeSelf,
513528
excludePackages = DEFAULT_EXCLUDE_PACKAGES,
529+
failOnError = false,
514530
...options
515531
}: FindNodeApiModuleOptions & {
516532
includeSelf: boolean;
517533
excludePackages?: string[];
534+
failOnError?: boolean;
518535
}) {
519536
// Find the location of each dependency
520-
const packagePathsByName = findPackageDependencyPaths(fromPath);
537+
const packagePathsByName = findPackageDependencyPaths(fromPath, {
538+
failOnError,
539+
});
521540
if (includeSelf) {
522541
const packageRoot = packageDirectorySync({ cwd: fromPath });
523542
assert(packageRoot, `Could not find package root from ${fromPath}`);

0 commit comments

Comments
 (0)