Skip to content

Conversation

@kraenhansen
Copy link
Collaborator

@kraenhansen kraenhansen commented Apr 28, 2025

Add an option to choose a naming strategy of XCFrameworks as they're vendored (i.e. copied and renamed) into the host package.

You can choose between two modes:

  • "hash": Will traverse the filesystem upwards from the Node-API xcframework directory and use the package name of the encapsulating package. This package name is then appended with the relative path to the xcframework and this combined path (package name + relative path) is then hashed and truncated. The advantage is that we have no restrictions on how many Node-API modules a dependency of an app can bring and we can even disambiguate between Node-API modules of the same name within a single package. The disadvantage is that the name of the framework is not easily translatable to the original Node-API module, which might make it harder to debug issues. (We could provide tools to scan and resolve the original file from the hash though 🤔)
  • "package-name": Will traverse the filesystem upwards from the Node-API xcframework directory and use the package name of the encapsulating package. The advantage being that names don't collide across dependencies of an app and the frameworks in Xcode and crashdumps will make more sense. The disadvantage is that each dependency of an app can bring at most one Node-API module.

The option defaults to package-name as I believe that strikes the right balance between DX and compatibility with the existing ecosystem og packages shipping Node-API modules (of which the vast majority ships only one per package).

You can opt-into the "hash" strategy if you need to, by running pod install with an enviroment variable:

NODE_API_MODULES_NAMING=hash npm run pod-install

and setting the naming option on the Babel plugin:

https://github.com/callstackincubator/react-native-node-api-modules/blob/9254501866f58510efe9c2e202d7feeeb2071ddf/apps/test-app/babel.config.js#L3

What it looks like

With the examples sub-packages renamed to example-${counter}, this is what it looks like to a dev:

Frameworks in Xcode

Screenshot 2025-04-28 at 12 20 16

Frameworks on disk

Screenshot 2025-04-28 at 12 20 31

@changeset-bot
Copy link

changeset-bot bot commented Apr 28, 2025

⚠️ No Changeset found

Latest commit: 8a4bf7b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kraenhansen kraenhansen self-assigned this Apr 28, 2025
@kraenhansen kraenhansen requested a review from shirakaba April 28, 2025 10:29
@teodorciuraru teodorciuraru requested review from shirakaba and teodorciuraru and removed request for shirakaba and teodorciuraru April 29, 2025 17:10
@teodorciuraru
Copy link

Only one reviewer per PR, or is it playing with me? 😂

I'll leave comments separately, no worries.

@kraenhansen kraenhansen requested review from teodorciuraru and removed request for shirakaba April 29, 2025 18:14
@kraenhansen
Copy link
Collaborator Author

kraenhansen commented Apr 29, 2025

Only one reviewer per PR, or is it playing with me?

No - you're right. I've changed it to you - it's first come first serve 😆
I believe it's because the repo is private - other features are limited by that too.

Base automatically changed from kh/package-name-and-relative-paths to main April 29, 2025 21:26
@kraenhansen kraenhansen force-pushed the kh/package-names-as-xcframework-names branch from 9254501 to eca9fb4 Compare April 29, 2025 21:28
@teodorciuraru
Copy link

Need more time on this one. Feel free to merge if needed.

Comment on lines +95 to +110
// export async function updateLibraryInstallPathInXCFramework(
// xcframeworkPath: string
// ) {
// for (const file of fs.readdirSync(xcframeworkPath, {
// withFileTypes: true,
// recursive: true,
// })) {
// if (file.isDirectory() && path.extname(file.name) === ".framework") {
// const libraryName = path.basename(file.name, ".framework");
// const libraryPath = path.join(file.parentPath, file.name, libraryName);
// assert(fs.existsSync(libraryPath), `Expected library at: ${libraryPath}`);
// const newInstallName = getLibraryInstallName(xcframeworkPath);
// await spawn("install_name_tool", ["-id", newInstallName, libraryPath]);
// }
// }
// }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably be deleted, but I'll let it stick around here for a bit longer in case it comes in handy 🙈

@kraenhansen
Copy link
Collaborator Author

kraenhansen commented May 2, 2025

I'll take what I can get 😉 I plan to merge this EOD. I have the Android support based on this and I'd like to refactor the CLI to have a single "xcframeworks" command with sub-commands, like "list", "copy", etc. I'd also like to get started on setting up CI to build and test all of this 🙏

@kraenhansen kraenhansen merged commit 287d0ad into main May 6, 2025
@kraenhansen kraenhansen deleted the kh/package-names-as-xcframework-names branch May 6, 2025 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants