fix: fix codegen not finding all third-party libraries

[tido64]

Summary:

pod install (and probably Android build as well) fails when New Architecture is enabled because the path to @react-native/codegen is hardcoded.

% pod install --project-directory=ios
Framework build type is static library
[Codegen] Adding script_phases to React-Codegen.
[Codegen] Generating ios/build/generated/ios/React-Codegen.podspec.json

[!] Invalid `Podfile` file: [!] node ios/../node_modules/react-native/scripts/generate-codegen-artifacts.js -p /~/packages/test-app -o /~/packages/test-app/ios -e true -c

[Codegen] Processing react-native core libraries
[Codegen] Found react-native


[Codegen] >>>>> Searching for codegen-enabled libraries in /~/node_modules/.store/react-native-virtual-0bcd9fdafa/node_modules


[Codegen] >>>>> Searching for codegen-enabled libraries in react-native.config.js


[Codegen] >>>>> Searching for codegen-enabled libraries in the app


[Codegen] Done.
error: Could not determine @react-native/codegen location. Try running 'yarn install' or 'npm install' in your project root.

Changelog:

[GENERAL] [FIXED] - Fix codegen not finding all third-party libraries

Test Plan:

git clone https://github.com/microsoft/rnx-kit.git
cd rnx-kit
yarn

# Apply the patch manually

cd packages/test-app
yarn build --dependencies
RCT_NEW_ARCH_ENABLED=1 pod install --project-directory=ios

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,500,358	-8,746,580
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	9,754,916	-10,854,421
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 6974697
Branch: main

[github-actions]

	Warnings
⚠️	Can't find the E2E test log for ci/circleci: test_e2e_ios. Job link

Generated by 🚫 dangerJS against 00bcaec

[jbroma]

@tido64 I've been experimenting with pnpm (or rather yarn with linker: pnpm), and I've also found out that except for this, codegen has also issues with finding codegen-enabled libraries as the path where it looks for them is wrong when not using classic node_modules structure. Did you run into this issue as well?

This part causes the issues for me:

function handleThirdPartyLibraries(
  libraries,
  baseCodegenConfigFileDir,
  dependencies,
  codegenConfigFilename,
  codegenConfigKey,
) {
  // Determine which of these are codegen-enabled libraries
  const configDir =
    baseCodegenConfigFileDir ||
    path.join(REACT_NATIVE_PACKAGE_ROOT_FOLDER, '..');
  console.log(
    `\n\n[Codegen] >>>>> Searching for codegen-enabled libraries in ${configDir}`,
  );

to fix it, I've used this:

const configDir =
    baseCodegenConfigFileDir ||
    path.join(process.argv[1], '..', '..', '..');

process.argv[1] is equal to node_modules/react-native/scripts/generate-codegen-artifacts.js in my case, which points to a symlinked react-native and not it's target location inside the store.

[tido64]

Did you run into this issue as well?

I wasn't paying attention, but now that you mention it, this is also an issue. I fixed it like this:

diff --git a/packages/react-native/scripts/codegen/generate-artifacts-executor.js b/packages/react-native/scripts/codegen/generate-artifacts-executor.js
index 5980da20893..3c1dbed7c15 100644
--- a/packages/react-native/scripts/codegen/generate-artifacts-executor.js
+++ b/packages/react-native/scripts/codegen/generate-artifacts-executor.js
@@ -197,7 +197,7 @@ function handleThirdPartyLibraries(
   // Determine which of these are codegen-enabled libraries
   const configDir =
     baseCodegenConfigFileDir ||
-    path.join(REACT_NATIVE_PACKAGE_ROOT_FOLDER, '..');
+    path.join(process.cwd(), 'node_modules');
   console.log(
     `\n\n[Codegen] >>>>> Searching for codegen-enabled libraries in ${configDir}`,
   );

But I'm not sure if this is sufficient. With a pnpm setup (pnpm or Yarn + pnpm), all dependencies are always present under node_modules. In a hoisted setup (npm, Yarn), libs may not be found here. Maybe someone from Meta can chime in. I'll be happy to include this in the PR.

FWIW, autolinking parses package.json and resolves from the project root so it doesn't matter where the dependencies are installed.

[NickGerleman]

Will let one of the releases folks merge this since it's going into release branch, but change lgtm.

Re structure, it seems like this is organized like a single app/library will depend on a single RN, then that RN dictates the association to version of codegen package? Is that right?

[tido64]

Re structure, it seems like this is organized like a single app/library will depend on a single RN, then that RN dictates the association to version of codegen package? Is that right?

Yes, that sounds right. On main, this code has changed to do proper require statements.

However, an issue still remains that it's looking for third-party libraries in the wrong place:

react-native/packages/react-native/scripts/codegen/generate-artifacts-executor.js

Lines 192 to 194 in 36a4a24

	const configDir =
	baseCodegenConfigFileDir ||
	path.join(REACT_NATIVE_PACKAGE_ROOT_FOLDER, '..');

It looks like it was addressed on main, but I am currently not able to test it. Looking at the code, I see it using an undefined variable, codegenConfigFileDir, which doesn't seem to come from anywhere.

Edit: I've updated the patch from above. I think this should be more or less correct:

diff --git a/packages/react-native/scripts/codegen/generate-artifacts-executor.js b/packages/react-native/scripts/codegen/generate-artifacts-executor.js
index 5980da20893..23faf9ca370 100644
--- a/packages/react-native/scripts/codegen/generate-artifacts-executor.js
+++ b/packages/react-native/scripts/codegen/generate-artifacts-executor.js
@@ -195,33 +195,34 @@ function handleThirdPartyLibraries(
   codegenConfigKey,
 ) {
   // Determine which of these are codegen-enabled libraries
-  const configDir =
-    baseCodegenConfigFileDir ||
-    path.join(REACT_NATIVE_PACKAGE_ROOT_FOLDER, '..');
+  const configDir = baseCodegenConfigFileDir || process.cwd();
   console.log(
     `\n\n[Codegen] >>>>> Searching for codegen-enabled libraries in ${configDir}`,
   );

   // Handle third-party libraries
+  const resolveOptions = {paths: [configDir]};
   Object.keys(dependencies).forEach(dependency => {
     if (dependency === REACT_NATIVE_DEPENDENCY_NAME) {
       // react-native should already be added.
       return;
     }
-    const codegenConfigFileDir = path.join(configDir, dependency);
-    const configFilePath = path.join(
-      codegenConfigFileDir,
-      codegenConfigFilename,
-    );
-    if (fs.existsSync(configFilePath)) {
+
+    try {
+      const configFilePath = require.resolve(
+        `${dependency}/${codegenConfigFilename}`,
+        resolveOptions,
+      );
       const configFile = JSON.parse(fs.readFileSync(configFilePath));
       extractLibrariesFromJSON(
         configFile,
         libraries,
         codegenConfigKey,
         dependency,
-        codegenConfigFileDir,
+        path.dirname(configFilePath),
       );
+    } catch (_) {
+      // ignore
     }
   });
 }

[tido64]

@dmytrorykun: What do you think of the patch here: #42943 (comment). Should it be included or should I open a new PR?

[dmytrorykun]

@tido64 it looks like these changes belong to this PR, yeah, let's include them.