diff --git a/.changeset/orange-bananas-obey.md b/.changeset/orange-bananas-obey.md new file mode 100644 index 00000000..68c5aa45 --- /dev/null +++ b/.changeset/orange-bananas-obey.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": minor +--- + +Scope is now stripped from package names when renaming libraries while linking diff --git a/apps/test-app/babel.config.js b/apps/test-app/babel.config.js index 8ea846e2..1eecb8fa 100644 --- a/apps/test-app/babel.config.js +++ b/apps/test-app/babel.config.js @@ -1,5 +1,5 @@ module.exports = { presets: ["module:@react-native/babel-preset"], - // plugins: [['module:react-native-node-api/babel-plugin', { stripPathSuffix: true }]], + // plugins: [['module:react-native-node-api/babel-plugin', { packageName: "strip", pathSuffix: "strip" }]], plugins: ["module:react-native-node-api/babel-plugin"], }; diff --git a/packages/host/src/node/babel-plugin/plugin.ts b/packages/host/src/node/babel-plugin/plugin.ts index 6b9937cb..45e269e9 100644 --- a/packages/host/src/node/babel-plugin/plugin.ts +++ b/packages/host/src/node/babel-plugin/plugin.ts @@ -9,11 +9,22 @@ import { isNodeApiModule, findNodeAddonForBindings, NamingStrategy, - PathSuffixChoice, - assertPathSuffix, + LibraryNamingChoice, + assertLibraryNamingChoice, } from "../path-utils"; export type PluginOptions = { + /** + * Controls how the package name is transformed into a library name. + * The transformation is needed to disambiguate and avoid conflicts between addons with the same name (but in different sub-paths or packages). + * + * As an example, if the package name is `@my-org/my-pkg` and the path of the addon within the package is `build/Release/my-addon.node` (and `pathSuffix` is set to `"strip"`): + * - `"omit"`: Only the path within the package is used and the library name will be `my-addon`. + * - `"strip"`: Scope / org gets stripped and the library name will be `my-pkg--my-addon`. + * - `"keep"`: The org and name is kept and the library name will be `my-org--my-pkg--my-addon`. + */ + packageName?: LibraryNamingChoice; + /** * Controls how the path of the addon inside a package is transformed into a library name. * The transformation is needed to disambiguate and avoid conflicts between addons with the same name (but in different sub-paths or packages). @@ -23,13 +34,16 @@ export type PluginOptions = { * - `"strip"` (default): Path gets stripped to its basename and the library name will be `my-pkg--my-addon`. * - `"keep"`: The full path is kept and the library name will be `my-pkg--build-Release-my-addon`. */ - pathSuffix?: PathSuffixChoice; + pathSuffix?: LibraryNamingChoice; }; function assertOptions(opts: unknown): asserts opts is PluginOptions { assert(typeof opts === "object" && opts !== null, "Expected an object"); if ("pathSuffix" in opts) { - assertPathSuffix(opts.pathSuffix); + assertLibraryNamingChoice(opts.pathSuffix); + } + if ("packageName" in opts) { + assertLibraryNamingChoice(opts.packageName); } } @@ -57,7 +71,7 @@ export function plugin(): PluginObj { visitor: { CallExpression(p) { assertOptions(this.opts); - const { pathSuffix = "strip" } = this.opts; + const { pathSuffix = "strip", packageName = "strip" } = this.opts; if (typeof this.filename !== "string") { // This transformation only works when the filename is known return; @@ -80,6 +94,7 @@ export function plugin(): PluginObj { const resolvedPath = findNodeAddonForBindings(id, from); if (typeof resolvedPath === "string") { replaceWithRequireNodeAddon(p.parentPath, resolvedPath, { + packageName, pathSuffix, }); } @@ -89,7 +104,10 @@ export function plugin(): PluginObj { isNodeApiModule(path.join(from, id)) ) { const relativePath = path.join(from, id); - replaceWithRequireNodeAddon(p, relativePath, { pathSuffix }); + replaceWithRequireNodeAddon(p, relativePath, { + packageName, + pathSuffix, + }); } } }, diff --git a/packages/host/src/node/cli/options.ts b/packages/host/src/node/cli/options.ts index 0944f7c9..eb059b42 100644 --- a/packages/host/src/node/cli/options.ts +++ b/packages/host/src/node/cli/options.ts @@ -1,15 +1,28 @@ import { Option } from "@react-native-node-api/cli-utils"; -import { assertPathSuffix, PATH_SUFFIX_CHOICES } from "../path-utils"; +import { + assertLibraryNamingChoice, + LIBRARY_NAMING_CHOICES, +} from "../path-utils"; -const { NODE_API_PATH_SUFFIX } = process.env; +const { NODE_API_PACKAGE_NAME, NODE_API_PATH_SUFFIX } = process.env; +if (typeof NODE_API_PACKAGE_NAME === "string") { + assertLibraryNamingChoice(NODE_API_PACKAGE_NAME); +} if (typeof NODE_API_PATH_SUFFIX === "string") { - assertPathSuffix(NODE_API_PATH_SUFFIX); + assertLibraryNamingChoice(NODE_API_PATH_SUFFIX); } +export const packageNameOption = new Option( + "--package-name ", + "Controls how the package name is transformed into a library name", +) + .choices(LIBRARY_NAMING_CHOICES) + .default(NODE_API_PACKAGE_NAME || "strip"); + export const pathSuffixOption = new Option( "--path-suffix ", "Controls how the path of the addon inside a package is transformed into a library name", ) - .choices(PATH_SUFFIX_CHOICES) + .choices(LIBRARY_NAMING_CHOICES) .default(NODE_API_PATH_SUFFIX || "strip"); diff --git a/packages/host/src/node/cli/program.ts b/packages/host/src/node/cli/program.ts index 37950d6d..d106cc74 100644 --- a/packages/host/src/node/cli/program.ts +++ b/packages/host/src/node/cli/program.ts @@ -23,7 +23,7 @@ import { } from "../path-utils"; import { command as vendorHermes } from "./hermes"; -import { pathSuffixOption } from "./options"; +import { packageNameOption, pathSuffixOption } from "./options"; import { linkModules, pruneLinkedModules, ModuleLinker } from "./link-modules"; import { linkXcframework } from "./apple"; import { linkAndroidDir } from "./android"; @@ -70,10 +70,14 @@ program ) .option("--android", "Link Android modules") .option("--apple", "Link Apple modules") + .addOption(packageNameOption) .addOption(pathSuffixOption) .action( wrapAction( - async (pathArg, { force, prune, pathSuffix, android, apple }) => { + async ( + pathArg, + { force, prune, pathSuffix, android, apple, packageName }, + ) => { console.log("Auto-linking Node-API modules from", chalk.dim(pathArg)); const platforms: PlatformName[] = []; if (android) { @@ -101,7 +105,7 @@ program platform, fromPath: path.resolve(pathArg), incremental: !force, - naming: { pathSuffix }, + naming: { packageName, pathSuffix }, linker: getLinker(platform), }), { @@ -173,9 +177,10 @@ program .description("Lists Node-API modules") .argument("[from-path]", "Some path inside the app package", process.cwd()) .option("--json", "Output as JSON", false) + .addOption(packageNameOption) .addOption(pathSuffixOption) .action( - wrapAction(async (fromArg, { json, pathSuffix }) => { + wrapAction(async (fromArg, { json, pathSuffix, packageName }) => { const rootPath = path.resolve(fromArg); const dependencies = await findNodeApiModulePathsByDependency({ fromPath: rootPath, @@ -210,7 +215,7 @@ program ); logModulePaths( dependency.modulePaths.map((p) => path.join(dependency.path, p)), - { pathSuffix }, + { packageName, pathSuffix }, ); } } @@ -222,21 +227,22 @@ program .description( "Utility to print, module path, the hash of a single Android library", ) + .addOption(packageNameOption) .addOption(pathSuffixOption) .action( - wrapAction((pathInput, { pathSuffix }) => { + wrapAction((pathInput, { pathSuffix, packageName }) => { const resolvedModulePath = path.resolve(pathInput); const normalizedModulePath = normalizeModulePath(resolvedModulePath); - const { packageName, relativePath } = - determineModuleContext(resolvedModulePath); + const context = determineModuleContext(resolvedModulePath); const libraryName = getLibraryName(resolvedModulePath, { + packageName, pathSuffix, }); console.log({ resolvedModulePath, normalizedModulePath, - packageName, - relativePath, + packageName: context.packageName, + relativePath: context.relativePath, libraryName, }); }), diff --git a/packages/host/src/node/path-utils.test.ts b/packages/host/src/node/path-utils.test.ts index cc6b307b..e780f802 100644 --- a/packages/host/src/node/path-utils.test.ts +++ b/packages/host/src/node/path-utils.test.ts @@ -208,6 +208,7 @@ describe("getLibraryName", () => { }); assert.equal( getLibraryName(path.join(tempDirectoryPath, "addon"), { + packageName: "keep", pathSuffix: "keep", }), "my-package--addon", @@ -215,6 +216,7 @@ describe("getLibraryName", () => { assert.equal( getLibraryName(path.join(tempDirectoryPath, "sub-directory/addon"), { + packageName: "keep", pathSuffix: "keep", }), "my-package--sub-directory-addon", @@ -230,6 +232,7 @@ describe("getLibraryName", () => { }); assert.equal( getLibraryName(path.join(tempDirectoryPath, "addon"), { + packageName: "keep", pathSuffix: "strip", }), "my-package--addon", @@ -237,6 +240,7 @@ describe("getLibraryName", () => { assert.equal( getLibraryName(path.join(tempDirectoryPath, "sub-directory", "addon"), { + packageName: "keep", pathSuffix: "strip", }), "my-package--addon", @@ -252,6 +256,7 @@ describe("getLibraryName", () => { }); assert.equal( getLibraryName(path.join(tempDirectoryPath, "addon"), { + packageName: "keep", pathSuffix: "omit", }), "my-package", @@ -259,11 +264,54 @@ describe("getLibraryName", () => { assert.equal( getLibraryName(path.join(tempDirectoryPath, "sub-directory", "addon"), { + packageName: "keep", pathSuffix: "omit", }), "my-package", ); }); + + it("keeps and escapes scope from package name", (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "package.json": `{ "name": "@my-org/my-package" }`, + "addon.apple.node/addon.node": "// This is supposed to be a binary file", + }); + assert.equal( + getLibraryName(path.join(tempDirectoryPath, "addon"), { + packageName: "keep", + pathSuffix: "strip", + }), + "my-org__my-package--addon", + ); + }); + + it("strips scope from package name", (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "package.json": `{ "name": "@my-org/my-package" }`, + "addon.apple.node/addon.node": "// This is supposed to be a binary file", + }); + assert.equal( + getLibraryName(path.join(tempDirectoryPath, "addon"), { + packageName: "strip", + pathSuffix: "strip", + }), + "my-package--addon", + ); + }); + + it("omits scope from package name", (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "package.json": `{ "name": "@my-org/my-package" }`, + "addon.apple.node/addon.node": "// This is supposed to be a binary file", + }); + assert.equal( + getLibraryName(path.join(tempDirectoryPath, "addon"), { + packageName: "omit", + pathSuffix: "strip", + }), + "addon", + ); + }); }); describe("findPackageDependencyPaths", () => { diff --git a/packages/host/src/node/path-utils.ts b/packages/host/src/node/path-utils.ts index 1fe6170e..79fcf241 100644 --- a/packages/host/src/node/path-utils.ts +++ b/packages/host/src/node/path-utils.ts @@ -18,22 +18,33 @@ export const PLATFORM_EXTENSIONS = { apple: ".apple.node", } as const satisfies Record; -export type PlatformExtentions = (typeof PLATFORM_EXTENSIONS)[PlatformName]; +export type PlatformExtensions = (typeof PLATFORM_EXTENSIONS)[PlatformName]; -export const PATH_SUFFIX_CHOICES = ["strip", "keep", "omit"] as const; -export type PathSuffixChoice = (typeof PATH_SUFFIX_CHOICES)[number]; +export const LIBRARY_NAMING_CHOICES = ["strip", "keep", "omit"] as const; +export type LibraryNamingChoice = (typeof LIBRARY_NAMING_CHOICES)[number]; -export function assertPathSuffix( +export function assertLibraryNamingChoice( value: unknown, -): asserts value is PathSuffixChoice { +): asserts value is LibraryNamingChoice { assert(typeof value === "string", `Expected a string, got ${typeof value}`); assert( - (PATH_SUFFIX_CHOICES as readonly string[]).includes(value), - `Expected one of ${PATH_SUFFIX_CHOICES.join(", ")}`, + (LIBRARY_NAMING_CHOICES as readonly string[]).includes(value), + `Expected one of ${LIBRARY_NAMING_CHOICES.join(", ")}`, ); } export type NamingStrategy = { + /** + * Controls how the package name is transformed into a library name. + * The transformation is needed to disambiguate and avoid conflicts between addons with the same name (but in different sub-paths or packages). + * + * As an example, if the package name is `@my-org/my-pkg` and the path of the addon within the package is `build/Release/my-addon.node` (and `pathSuffix` is set to `"strip"`): + * - `"omit"`: Only the path within the package is used and the library name will be `my-addon`. + * - `"strip"`: Scope / org gets stripped and the library name will be `my-pkg--my-addon`. + * - `"keep"`: The org and name is kept and the library name will be `my-org--my-pkg--my-addon`. + */ + packageName: LibraryNamingChoice; + /** * Controls how the path of the addon inside a package is transformed into a library name. * The transformation is needed to disambiguate and avoid conflicts between addons with the same name (but in different sub-paths or packages). @@ -43,7 +54,7 @@ export type NamingStrategy = { * - `"strip"`: Path gets stripped to its basename and the library name will be `my-pkg--my-addon`. * - `"keep"`: The full path is kept and the library name will be `my-pkg--build-Release-my-addon`. */ - pathSuffix: PathSuffixChoice; + pathSuffix: LibraryNamingChoice; }; // Cache mapping package directory to package name across calls @@ -176,22 +187,71 @@ export function normalizeModulePath(modulePath: string) { } export function escapePath(modulePath: string) { - return modulePath.replace(/[^a-zA-Z0-9]/g, "-"); + return ( + modulePath + // Replace any non-alphanumeric character with a dash + .replace(/[^a-zA-Z0-9-_]/g, "-") + ); +} + +export function transformPackageName( + packageName: string, + strategy: LibraryNamingChoice, +) { + if (strategy === "omit") { + return ""; + } else if (packageName.startsWith("@")) { + const [first, ...rest] = packageName.split("/"); + assert(rest.length > 0, `Invalid scoped package name (${packageName})`); + if (strategy === "strip") { + return escapePath(rest.join("/")); + } else { + // Stripping away the @ and using double underscore to separate scope and name is common practice in other projects (like DefinitelyTyped) + return escapePath(`${first.replace(/^@/, "")}__${rest.join("/")}`); + } + } else { + return escapePath(packageName); + } +} + +export function transformPathSuffix( + relativePath: string, + strategy: LibraryNamingChoice, +) { + if (strategy === "omit") { + return ""; + } else if (strategy === "strip") { + return escapePath(path.basename(relativePath)); + } else { + return escapePath(relativePath.replaceAll(/[/\\]/g, "-")); + } } /** * Get the name of the library which will be used when the module is linked in. */ export function getLibraryName(modulePath: string, naming: NamingStrategy) { + assert( + naming.packageName !== "omit" || naming.pathSuffix !== "omit", + "Both packageName and pathSuffix cannot be 'omit' at the same time", + ); const { packageName, relativePath } = determineModuleContext(modulePath); - const escapedPackageName = escapePath(packageName); - return naming.pathSuffix === "omit" - ? escapedPackageName - : `${escapedPackageName}--${escapePath( - naming.pathSuffix === "strip" - ? path.basename(relativePath) - : relativePath, - )}`; + const transformedPackageName = transformPackageName( + packageName, + naming.packageName, + ); + const transformedRelativePath = transformPathSuffix( + relativePath, + naming.pathSuffix, + ); + const parts = []; + if (transformedPackageName) { + parts.push(transformedPackageName); + } + if (transformedRelativePath) { + parts.push(transformedRelativePath); + } + return parts.join("--"); } export function prettyPath(p: string) {