Skip to content

Commit d5766c7

Browse files
authored
Replace path hashing with escaped relative paths (#32)
* Replace "hash" paths with path suffix * Create tsconfig for tests * Move "install name" logic into TurboModule
1 parent 287d0ad commit d5766c7

File tree

13 files changed

+503
-507
lines changed

13 files changed

+503
-507
lines changed

apps/test-app/babel.config.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module.exports = {
22
presets: ['module:@react-native/babel-preset'],
3-
// plugins: [['module:react-native-node-api-modules/babel-plugin', { naming: "hash" }]],
4-
plugins: [['module:react-native-node-api-modules/babel-plugin', { naming: "package-name" }]],
3+
// plugins: [['module:react-native-node-api-modules/babel-plugin', { stripPathSuffix: true }]],
4+
plugins: ['module:react-native-node-api-modules/babel-plugin'],
55
};

packages/react-native-node-api-modules/cpp/CxxNodeApiHostModule.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,17 @@ CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
2323
return jsi::Value::undefined();
2424
}
2525

26-
jsi::Value CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
27-
const jsi::String path) {
28-
const std::string pathStr = path.utf8(rt);
26+
jsi::Value
27+
CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
28+
const jsi::String libraryName) {
29+
const std::string libraryNameStr = libraryName.utf8(rt);
2930

30-
auto [it, inserted] = nodeAddons_.emplace(pathStr, NodeAddon());
31+
auto [it, inserted] = nodeAddons_.emplace(libraryNameStr, NodeAddon());
3132
NodeAddon &addon = it->second;
3233

3334
// Check if this module has been loaded already, if not then load it...
3435
if (inserted) {
35-
if (!loadNodeAddon(addon, pathStr)) {
36+
if (!loadNodeAddon(addon, libraryNameStr)) {
3637
return jsi::Value::undefined();
3738
}
3839
}
@@ -47,10 +48,19 @@ jsi::Value CxxNodeApiHostModule::requireNodeAddon(jsi::Runtime &rt,
4748
}
4849

4950
bool CxxNodeApiHostModule::loadNodeAddon(NodeAddon &addon,
50-
const std::string &path) const {
51+
const std::string &libraryName) const {
52+
#if defined(__APPLE__)
53+
std::string libraryPath =
54+
"@rpath/" + libraryName + ".framework/" + libraryName;
55+
#elif defined(__ANDROID__)
56+
std::string libraryPath = libraryName
57+
#else
58+
abort()
59+
#endif
60+
5161
typename LoaderPolicy::Symbol initFn = NULL;
5262
typename LoaderPolicy::Module library =
53-
LoaderPolicy::loadLibrary(path.c_str());
63+
LoaderPolicy::loadLibrary(libraryPath.c_str());
5464
if (NULL != library) {
5565
addon.moduleHandle = library;
5666

packages/react-native-node-api-modules/react-native-node-api-modules.podspec

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ require_relative "./scripts/patch-hermes"
66

77
NODE_PATH ||= `which node`.strip
88
CLI_COMMAND ||= "'#{NODE_PATH}' '#{File.join(__dir__, "dist/node/cli/run.js")}'"
9-
COPY_FRAMEWORKS_COMMAND ||= "#{CLI_COMMAND} copy-xcframeworks '#{Pod::Config.instance.installation_root}'"
9+
STRIP_PATH_SUFFIX ||= ENV['NODE_API_MODULES_STRIP_PATH_SUFFIX'] === "true"
10+
COPY_FRAMEWORKS_COMMAND ||= "#{CLI_COMMAND} xcframeworks copy --podfile '#{Pod::Config.instance.installation_root}' #{STRIP_PATH_SUFFIX ? '--strip-path-suffix' : ''}"
1011

1112
# We need to run this now to ensure the xcframeworks are copied vendored_frameworks are considered
1213
XCFRAMEWORKS_DIR ||= File.join(__dir__, "xcframeworks")
1314
unless defined?(@xcframeworks_copied)
15+
puts "Executing #{COPY_FRAMEWORKS_COMMAND}"
1416
system(COPY_FRAMEWORKS_COMMAND) or raise "Failed to copy xcframeworks"
1517
# Setting a flag to avoid running this command on every require
1618
@xcframeworks_copied = true

packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { transformFileSync } from "@babel/core";
66

77
import { plugin } from "./plugin.js";
88
import { setupTempDirectory } from "../test-utils.js";
9-
import { getLibraryInstallName } from "../path-utils.js";
9+
import { getLibraryName } from "../path-utils.js";
1010

1111
describe("plugin", () => {
1212
it("transforms require calls, regardless", (context) => {
@@ -38,19 +38,19 @@ describe("plugin", () => {
3838
`,
3939
});
4040

41-
const ADDON_1_REQUIRE_ARG = getLibraryInstallName(
41+
const ADDON_1_REQUIRE_ARG = getLibraryName(
4242
path.join(tempDirectoryPath, "addon-1"),
43-
"hash"
43+
{ stripPathSuffix: false }
4444
);
45-
const ADDON_2_REQUIRE_ARG = getLibraryInstallName(
45+
const ADDON_2_REQUIRE_ARG = getLibraryName(
4646
path.join(tempDirectoryPath, "addon-2"),
47-
"hash"
47+
{ stripPathSuffix: false }
4848
);
4949

5050
{
5151
const result = transformFileSync(
5252
path.join(tempDirectoryPath, "./addon-1.js"),
53-
{ plugins: [[plugin, { naming: "hash" }]] }
53+
{ plugins: [[plugin, { stripPathSuffix: false }]] }
5454
);
5555
assert(result);
5656
const { code } = result;

packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,22 @@ import type { PluginObj, NodePath } from "@babel/core";
55
import * as t from "@babel/types";
66

77
import {
8-
getLibraryInstallName,
8+
getLibraryName,
99
isNodeApiModule,
1010
replaceWithNodeExtension,
1111
NamingStrategy,
12-
NAMING_STATEGIES,
1312
} from "../path-utils";
1413

1514
type PluginOptions = {
16-
naming?: NamingStrategy;
15+
stripPathSuffix?: boolean;
1716
};
1817

1918
function assertOptions(opts: unknown): asserts opts is PluginOptions {
2019
assert(typeof opts === "object" && opts !== null, "Expected an object");
21-
if ("naming" in opts) {
22-
assert(typeof opts.naming === "string", "Expected 'naming' to be a string");
20+
if ("stripPathSuffix" in opts) {
2321
assert(
24-
NAMING_STATEGIES.includes(opts.naming as NamingStrategy),
25-
"Expected 'naming' to be either 'hash' or 'package-name'"
22+
typeof opts.stripPathSuffix === "boolean",
23+
"Expected 'stripPathSuffix' to be a boolean"
2624
);
2725
}
2826
}
@@ -32,7 +30,7 @@ export function replaceWithRequireNodeAddon(
3230
modulePath: string,
3331
naming: NamingStrategy
3432
) {
35-
const requireCallArgument = getLibraryInstallName(
33+
const requireCallArgument = getLibraryName(
3634
replaceWithNodeExtension(modulePath),
3735
naming
3836
);
@@ -54,7 +52,7 @@ export function plugin(): PluginObj {
5452
visitor: {
5553
CallExpression(p) {
5654
assertOptions(this.opts);
57-
const { naming = "package-name" } = this.opts;
55+
const { stripPathSuffix = false } = this.opts;
5856
if (typeof this.filename !== "string") {
5957
// This transformation only works when the filename is known
6058
return;
@@ -77,15 +75,17 @@ export function plugin(): PluginObj {
7775
const relativePath = path.join(from, id);
7876
// TODO: Support traversing the filesystem to find the Node-API module
7977
if (isNodeApiModule(relativePath)) {
80-
replaceWithRequireNodeAddon(p.parentPath, relativePath, naming);
78+
replaceWithRequireNodeAddon(p.parentPath, relativePath, {
79+
stripPathSuffix,
80+
});
8181
}
8282
}
8383
} else if (
8484
!path.isAbsolute(id) &&
8585
isNodeApiModule(path.join(from, id))
8686
) {
8787
const relativePath = path.join(from, id);
88-
replaceWithRequireNodeAddon(p, relativePath, naming);
88+
replaceWithRequireNodeAddon(p, relativePath, { stripPathSuffix });
8989
}
9090
}
9191
},

packages/react-native-node-api-modules/src/node/cli/helpers.ts

Lines changed: 15 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { spawn } from "bufout";
88
import { packageDirectorySync } from "pkg-dir";
99
import { readPackageSync } from "read-pkg";
1010

11-
import { NamingStrategy, hashModulePath } from "../path-utils.js";
11+
import { NamingStrategy, getLibraryName } from "../path-utils.js";
1212

1313
// Must be in all xcframeworks to be considered as Node-API modules
1414
export const MAGIC_FILENAME = "react-native-node-api-module";
@@ -182,16 +182,8 @@ type RebuildXcframeworkOptions = {
182182
type VendoredXcframework = {
183183
originalPath: string;
184184
outputPath: string;
185-
} & (
186-
| {
187-
hash: string;
188-
packageName?: never;
189-
}
190-
| {
191-
hash?: never;
192-
packageName: string;
193-
}
194-
);
185+
libraryName: string;
186+
};
195187

196188
type VendoredXcframeworkResult = VendoredXcframework & {
197189
skipped: boolean;
@@ -201,24 +193,16 @@ export function determineVendoredXcframeworkDetails(
201193
modulePath: string,
202194
naming: NamingStrategy
203195
): VendoredXcframework {
204-
if (naming === "hash") {
205-
const hash = hashModulePath(modulePath);
206-
return {
207-
hash,
208-
originalPath: modulePath,
209-
outputPath: path.join(XCFRAMEWORKS_PATH, `node-api-${hash}.xcframework`),
210-
};
211-
} else {
212-
const packageRoot = packageDirectorySync({ cwd: modulePath });
213-
assert(packageRoot, `Could not find package root from ${modulePath}`);
214-
const { name } = readPackageSync({ cwd: packageRoot });
215-
assert(name, `Could not find package name from ${packageRoot}`);
216-
return {
217-
packageName: name,
218-
originalPath: modulePath,
219-
outputPath: path.join(XCFRAMEWORKS_PATH, `${name}.xcframework`),
220-
};
221-
}
196+
const packageRoot = packageDirectorySync({ cwd: modulePath });
197+
assert(packageRoot, `Could not find package root from ${modulePath}`);
198+
const { name } = readPackageSync({ cwd: packageRoot });
199+
assert(name, `Could not find package name from ${packageRoot}`);
200+
const libraryName = getLibraryName(modulePath, naming);
201+
return {
202+
originalPath: modulePath,
203+
outputPath: path.join(XCFRAMEWORKS_PATH, `${libraryName}.xcframework`),
204+
libraryName,
205+
};
222206
}
223207

224208
export function hasDuplicatesWhenVendored(
@@ -243,13 +227,8 @@ export async function vendorXcframework({
243227
}: RebuildXcframeworkOptions): Promise<VendoredXcframeworkResult> {
244228
// Copy the xcframework to the output directory and rename the framework and binary
245229
const details = determineVendoredXcframeworkDetails(modulePath, naming);
246-
const { outputPath } = details;
247-
const discriminator =
248-
typeof details.hash === "string" ? details.hash : details.packageName;
249-
const tempPath = path.join(
250-
XCFRAMEWORKS_PATH,
251-
`node-api-${discriminator}-temp`
252-
);
230+
const { outputPath, libraryName: newLibraryName } = details;
231+
const tempPath = path.join(XCFRAMEWORKS_PATH, `${newLibraryName}.temp`);
253232
try {
254233
if (incremental && existsSync(outputPath)) {
255234
const moduleModified = getLatestMtime(modulePath);
@@ -284,10 +263,6 @@ export async function vendorXcframework({
284263
".framework"
285264
);
286265
const oldLibraryPath = path.join(frameworkPath, oldLibraryName);
287-
const newLibraryName = path.basename(
288-
details.outputPath,
289-
".xcframework"
290-
);
291266
const newFrameworkPath = path.join(
292267
tripletPath,
293268
`${newLibraryName}.framework`

0 commit comments

Comments
 (0)