Skip to content

[DRAFT] Make Babel plugin more "compatible" with Node.js; emit requireNodeAddons() with 3 args #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 24 commits into from
Closed

[DRAFT] Make Babel plugin more "compatible" with Node.js; emit requireNodeAddons() with 3 args #105

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0a106e5
feat(babel): add 2 params to requireNodeAddon(); respect package entr…
May 20, 2025
6725125
refactor: extract `findNodeAddonForBindings()`
May 20, 2025
c64891f
feat: add shortcut to `isNodeApiModule()`
May 20, 2025
2d17eb6
feat: relax the condition for CJS modules without file exts
May 21, 2025
8bb3e77
style: address linter issues
May 21, 2025
0ed5259
feat: extract path normalization from `determineModuleContext()`
Jun 4, 2025
4dcf2fd
feat: update tests to cover `determineNormalizedModuleContext()`
Jun 4, 2025
9bb9eec
feat: add test for scoped package names
Jun 4, 2025
4154615
feat: update tests for 3 arg `requireNodeAddon()`
Jun 4, 2025
ee87dd0
feat: remove xcframework dir from test code
Jun 4, 2025
a60c848
feat: change the third param to original path
Jun 4, 2025
41d4ebb
feat: add test case for require with "main" entry point
Jun 4, 2025
5596273
fix: remove unused `requiredFrom` parameter
Jun 4, 2025
d23fe0b
fix: remove unsupported babel option "naming"
Jun 4, 2025
63725af
feat: add tests for `findNodeAddonsForBindings()`
Jun 4, 2025
25b55da
fix: ensure that module paths use only forward slash (esp. on Windows)
Jun 4, 2025
e5c28a7
feat: add test case for addon that "escaped" its package
Jun 4, 2025
7106775
feat: make `resolvedPath` optional as suggested by Kraen
Jun 11, 2025
47db302
feat: run each test case as separate call to `it()`
Jun 11, 2025
8e1c278
chore: move `findNodeAddonForBindings()` to path-utils
Jun 11, 2025
4aa8549
chore: rename `originalPath` parameter to `originalId`
Jun 11, 2025
a431e8e
chore: replace `[].forEach()` with for-of
Jun 11, 2025
446b1a4
feat: move path slash normalization to determineModuleContext()
Jun 11, 2025
cdf1316
chore: use more descriptive variable names
Jun 11, 2025
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
7 changes: 4 additions & 3 deletions packages/react-native-node-api-modules/src/node/babel-plugin/plugin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ describe("plugin", () => {
inputFile: string;
};

([
const testCases: ReadonlyArray<TestCaseParams> = [
{ resolvedPath: "./addon-1.node", originalPath: "./addon-1.node", inputFile: "./addon-1.js" },
{ resolvedPath: "./addon-2.node", originalPath: "./addon-2.node", inputFile: "./addon-2.js" },
{ resolvedPath: "./addon-1.node", originalPath: "../addon-1.node", inputFile: "./sub-directory/addon-1.js" },
{ resolvedPath: "./addon-2.node", originalPath: "../addon-2.node", inputFile: "./sub-directory-3/addon-outside.js" },
{ resolvedPath: "./addon-1.node", originalPath: "addon-1", inputFile: "./addon-1-bindings.js" },
{ resolvedPath: undefined, originalPath: "./addon-1.js", inputFile: "./require-js-file.js" },
] as TestCaseParams[]).forEach(({ resolvedPath, originalPath, inputFile }) => {
];
for (const { resolvedPath, originalPath, inputFile } of testCases) {
const expectedMessage = resolvedPath
? `transform to requireNodeAddon() with "${resolvedPath}"`
: "NOT transform to requireNodeAddon()";
Expand Down Expand Up @@ -80,7 +81,7 @@ describe("plugin", () => {
);
}
});
});
}
});

it("transforms require calls to packages with native entry point", (context) => {
Expand Down
7 changes: 3 additions & 4 deletions packages/react-native-node-api-modules/src/node/babel-plugin/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ function tryResolveModulePath(id: string, from: string): string | undefined {
export function replaceWithRequireNodeAddon3(
p: NodePath,
resolvedPath: string,
originalPath: string
originalId: string
) {
const { packageName, relativePath } = determineModuleContext(resolvedPath);
const relPath = relativePath.replaceAll("\\", "/");
const finalRelPath = relPath.startsWith("./") ? relPath : `./${relPath}`;
const dotRelativePath = relativePath.startsWith("./") ? relativePath : `./${relativePath}`;

p.replaceWith(
t.callExpression(
Expand All @@ -71,7 +70,7 @@ export function replaceWithRequireNodeAddon3(
]),
t.identifier("requireNodeAddon")
),
[finalRelPath, packageName, originalPath]
[dotRelativePath, packageName, originalId]
.map(t.stringLiteral),
)
);
Expand Down
5 changes: 3 additions & 2 deletions packages/react-native-node-api-modules/src/node/path-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ export function determineModuleContext(
packageNameCache.set(pkgDir, pkgName);
}
// Compute module-relative path
const relPath = path.relative(pkgDir, originalPath);
return { packageName: pkgName, relativePath: relPath };
const relativePath = path.relative(pkgDir, originalPath)
.replaceAll("\\", "/");
Copy link
Collaborator

@kraenhansen kraenhansen Jun 12, 2025

Choose a reason for hiding this comment

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

Not feeling strong for this, but its a bit more semantic to use the constants:

Suggested change
.replaceAll("\\", "/");
.replaceAll(path.win32.sep, path.poxis.sep);

Copy link
Collaborator

@kraenhansen kraenhansen Jun 12, 2025

Choose a reason for hiding this comment

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

Might be good to have a test for this too?

return { packageName: pkgName, relativePath };
}

/**
Expand Down