Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/strict-link-resolution.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions docs/CLI.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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.
- `--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.

Expand Down
70 changes: 70 additions & 0 deletions packages/host/src/node/cli/bin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
});
});
5 changes: 4 additions & 1 deletion packages/host/src/node/cli/link-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -63,12 +64,14 @@ export async function linkModules({
naming,
platform,
linker,
failOnError,
}: LinkModulesOptions): Promise<ModuleOutput[]> {
// Find all their xcframeworks
const dependenciesByName = await findNodeApiModulePathsByDependency({
fromPath,
platform,
includeSelf: true,
failOnError,
});

// Find absolute paths to xcframeworks
Expand Down
10 changes: 9 additions & 1 deletion packages/host/src/node/cli/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -94,6 +101,7 @@ program
fromPath: path.resolve(pathArg),
naming: { packageName, pathSuffix },
linker: await createLinker(platform),
failOnError,
}),
{
text: `Linking ${platformDisplayName} Node-API modules`,
Expand Down
27 changes: 23 additions & 4 deletions packages/host/src/node/path-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -356,6 +363,7 @@ export function findPackageConfigurationByPath(
*/
export function findPackageDependencyPaths(
fromPath: string,
{ failOnError = false }: { failOnError?: boolean } = {},
): Record<string, string> {
const packageRoot = packageDirectorySync({ cwd: fromPath });
assert(packageRoot, `Could not find package root from ${fromPath}`);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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}`);
Expand Down