feat: Include transitive deps in autolinking

[TMisiukiewicz]

Summary:

Original issue: #870

Currently autolinking mechanism is using only dependencies explicitly defined in package.json. However, when you install package dependent on other package containing native code, these additional dependencies are not autolinked and developer needs to install them separately to make them visible for autolinking mechanism.

The script is going through all the dependencies recursively, collecting them into a single list with info if they contain any peer dependencies declared. Then, it removes all the peer dependencies that are already declared in package.json and those peer dependencies which do not contain any native code. It displays the list of all dependencies where any peer dependency with native code is missing and asks to install them. It gets the highest possible version of the library based on given ranges declared in peer dependencies and adds the library under dependencies with caret range. If it can't find any version that satisfies all the ranges, it displays warning and skips this library from being installed. After installation, it asks to install pods again. After installing it, the process of running/building an app continues.

At this moment I hide it behind a --dependency-check flag to make sure it's optional for developers. Enabling it for everyone would block CI pipelines where any dependencies are missing. This also opens up a possibility to test it for app developers and library maintainers.

transitive-deps.mov

To improve that flow I created a function that goes through dependencies recursively, collects all the dependencies regardless if they are introduced in app's dependencies, and additionally looks for its duplicates with different version. If there are any duplicates, the latest version is picked for autolinking.

This way libraries can declare packages dependent on other packages containing native code in dependencies. No matter if the latest version of the library is on the top-level of node_modules or in a nested one, the script will find it and pass the correct path to the config object.

Test Plan:

1. Set up the repository following the contributing guide
2. Create a fresh React Native project using CLI
3. Install the following packages using yarn/npm: @react-navigation/native @react-navigation/stack @gorhom/bottom-sheet react-navigation-tabs. The react-navigation-tabs is necessary for testing incompatibility between peer dependencies versions
4. Run yarn ios --dependency-check / npm run ios --dependency-check to verify if there are any dependencies containing native code that need to be installed.
5. Verify that the list contains @react-navigation/elements which is not a direct dependency of an app, but it's a dependency of @react-navigation/stack and contains react-native-safe-area-context as peer dependency.
6. Confirm installing missing dependencies
7. Verify that you have been warned about incompatibility of react-native-reanimated version
8. Verify that all the others installed libraries are now under dependencies in package.json
9. Confirm pod installation to make sure transitive dependencies are linked properly
10. After going through all these steps, the process will continue.

The same process will also work for build-ios, run-android and build-android commands. You can also test it on an existing app, to see if there are any peer dependencies containing native code that are not visible for autolinking mechanism.

Checklist

• npm support
• yarn support
• hide dependency check behind the flag
• write test plan
• Native code compatibility checks - to be investigated (possibly as a separate task)
• Write/update tests
• Documentation is up to date to reflect these changes.
• Follows commit message convention described in CONTRIBUTING.md

[satya164]

I think this approach is problematic. Let's consider how it would work in practice:

1. Native code can only have a single version
2. JS code can have multiple versions because Yarn/npm and bundlers support it

How it works now: Libraries define peer dependencies for native deps - it ensures user installs the dep which results in a single version being installed
The problem: User needs to install the native dep manually (though npm autoinstalls them now)

What this approach would do: Libraries define regular dependency for native deps, it solves the problem of user needing to install the dep manually
The problem: It introduces a new issue, since native dep is now a regular dependency, it can have multiple versions.

Now, it tries to circumvent the problem by autolinking only latest version. But it's problematic:

• Autolinking only works for native code, multiple versions of JS code will still be bundled to the app.
• Native code and JS code are tightly coupled. There's no rule that a different version of JS code would work with a different version of native code even if it's just a patch version different.

Different major versions of the native lib is a different issue, and I think that problem would exist in any approach. But the native lib not being compatible with itself due to slightly different version is a much bigger issue and can cause hard to debug bugs.

In my opinion, this is not a autolinking problem. They way autolinking works now is fine. This is a dependency management problem.

In the linked issue, I had proposed 2 approaches, both of them were about ensuring a single version of the library:

1. By automatically installing peer deps as regular dep to the app
2. By prompting to add overrides/resolutions to force a single version

See that both of these were about how to manage dependencies so that a single version is installed. Not specifically about linking a single version.

Quite some time has passed since I had proposed those approaches. Today I wouldn't propose the second approach at all. I think using peerDependencies for native libs is the correct approach. And whatever changes we do should be about how to make that experience better.

[TMisiukiewicz]

Using peerDependencices was my initial idea, however decided to give the approach above a try since yarn is not installing peer dependencies by default. However, if we decide to use peerDependencies, we could think about doing one of those:

1. New command in the CLI to install packages - if there are any peer dependencies, it would ask if developer wants to install it. However I'm not fully convinced to introduce a new command in CLI for that, since people are already used to install dependencies using yarn/npm, so it would bring a lot of confusion
2. When running any command, silently look for peer dependencies and inform user that some of them are missing -> ask to install them -> after installation remind about running pod install again.

But if we decide to install peer dependencies, should we introduce them to project's dependencies as well? I think it could be handy for other developers who won't need to go through peer deps installation process again

[TMisiukiewicz]

Here's the example demo of the workflow using peer dependencies:

Screen.Recording.2023-08-16.at.15.47.50.mov

Do you think picking highest compatible version and adding it to package.json with caret range will be safe to use?

[satya164]

New command in the CLI to install packages - if there are any peer dependencies, it would ask if developer wants to install it. However I'm not fully convinced to introduce a new command in CLI for that, since people are already used to install dependencies using yarn/npm, so it would bring a lot of confusion
When running any command, silently look for peer dependencies and inform user that some of them are missing -> ask to install them -> after installation remind about running pod install again.

I think checking it for run-x and build-x commands may be enough. we could also run pod install automatically after installing the deps.

But if we decide to install peer dependencies, should we introduce them to project's dependencies as well?

Yes, it needs to be in package.json, otherwise lockfile won't be correct.

Do you think picking highest compatible version and adding it to package.json with caret range will be safe to use?

It is safe if it matches all caret ranges. But also worth noting that sometimes some packages may not match the latest version, so we also need to think what to do if we can't find a version that matches all of them. Maybe we can install the latest one anyway and print a warning, or throw an error.

Also I think it'll be useful to show the version numbers for the packages in the prompt message.

[tomekzaw]

Hello everyone,

first of all, thanks @TMisiukiewicz for submitting this proposal and @satya164 for your comments which I truly agree with. I wanted to tell the story from the perspective of a third-party library, so here's how it works in react-native-reanimated.

Some time ago we found out that the vast majority of issues related to initialization and missing functions or JSI bindings was in fact caused by a mismatch of JavaScript code with compiled native code. In some cases, the problem was that the users would bump Reanimated and reinstall yarn dependencies but forgot to reinstall Pods and/or build the app again. As a result, the app used JS code from the new version while the native part of the app was still running on old binaries.

However, in some cases, there was indeed more than version of Reanimated installed within a single app. For instance, there was node_modules/react-native-reanimated with Reanimated 2.12.0 as well as node_modules/some-other-library/node_modules/react-native-reanimated with let's say Reanimated 2.9.0. This means that some-other-library will use JS code from Reanimated 2.9.0 (and the rest of the app will use JS code from 2.12.0) and thus will be incompatible with the native part of the app, i.e. NativeModules and binaries from C++ part of Reanimated which come from 2.12.0 (since autolinking works only for direct dependencies). Also, there can be at most one version of NativeModule actually installed because there can't be two Java classes with the same name.

The approach we chose was to inject the version into JS code and C++ code and compare them in runtime during initialization of Reanimated. So, the version of the library from package.json would be read by build.gradle and passed to CMakeLists.txt as a preprocessor flag and then converted into a jsi::String and injected to the main RN runtime using JSI. Then, on the JavaScript side we would also read the version from package.json and compare it with the value injected from C++ side.

Adding this check with a proper troubleshooting instructions in the error message saved hours of developers' time, both our users' and ourselves. At some point we decided to do the same for worklets (which need to be processed with our Babel plugin) and recently we decided to extend this mechanism for Java sources as well (which are distributed as .class files), see software-mansion/react-native-reanimated#4914.

Therefore, I think it would be really great to have these checks on the CLI side as well, especially considering that it would work the same way for all libraries with native code which do not have such checks (yet). However, I'm afraid that most of the libraries wouldn't support any changes related to paths (i.e. installing them from nested node_modules). Honestly, I like how things are organized now (in terms of declaring dependencies and filesystem) and many RN developers are already used to it. So what we really need is probably some check in the CLI (similar to what Expo CLI already does) as well as clear and detailed instructions on how to declare dependencies on libraries with native code and how to resolve conflicts.

Thanks again for working on this. Always happy to help!

[TMisiukiewicz]

Hi @tomekzaw, thank you for your insightful comment! This kind of checks is definitely something that would be highly beneficial. I'll add it to the checklist in the summary and investigate the possibilities of adding it to the CLI.

A quick update about current status of the work: Support for npm seems to work now but needs more testing. The flow is:

1. Recursively get all the dependencies
2. Since npm installs peer dependencies automatically, it's possible to go through node_modules and check if a dependency contains native code - basically we are looking for ios and android folders inside. Since we are about to introduce those dependencies in package.json, we should ignore all the peer deps without native code.
3. It gets all the occurencies of a missing peer dependency and based on the ranges, it looks for highest possible version to install
4. After installation, it checks if the package.json changed and asks to install pods again

Here's how it looks in the example - note that @react-navigation/elements is not in package.json (it's a dependency of @react-navigation/stack) and it contains react-native-safe-area-context as a peer dependency. After installation, react-native-safe-area-context is under dependencies in package.json and automatically becomes visible for autolinking mechanism.

Screen.Recording.2023-08-25.at.10.59.42.mov

Supporting yarn might get a little bit trickier, since it's not installing peer deps automatically so it's not possible to check which peer dependencies contain native code and need to be installed as a dependency. Couldn't find a proper solution for that so I'm thinking about installing all missing peer dependencies and overwriting the package.json and yarn.lock with the previous values. This will make the packages visible in the node_modules, however will make an experience a bit worse since it will take more time. Happy to hear suggestions how to handle it better if someone has some ideas.

If we get this merged, I think we should first hide it behind a flag for some time, so developers can test it and leave some initial feedback. It would also prevent us from failing actions on CIs using run-* or build-* commands.

[liamjones]

I assume you'd still be able to disable auto-linking for transitive native dependencies via react-native.config.js if this was merged to the CLI?

[TMisiukiewicz]

I assume you'd still be able to disable auto-linking for transitive native dependencies via react-native.config.js if this was merged to the CLI?

Hi @liamjones could you elaborate more how this would work? At this moment it'll be hidden behind --dependency-check flag to make this process optional

[liamjones]

Hi @liamjones could you elaborate more how this would work?

Sure @TMisiukiewicz.

Imagine a monorepo containing two RN apps A and B. They have a shared module S that they both depend on.

S uses a hypothetical native dependency, react-native-nd, in some parts of its code (but not all).

App A uses the parts of S that exercise react-native-nd, so need it to be autolinked.
App B doesn't use the parts of S that exercise react-native-nd, so, ideally, it doesn't want the overhead of the linked native module.

The autolinking docs explain you can customise the linking process to disable some autolinking.

So, if I make a react-native.config.js in app B containing:

module.exports = {
  dependencies: {
    'react-native-nd': {
      platforms: {
        android: null,
        ios: null
      },
    },
  },
};

And then ran yarn ios --dependency-check from app B would the react-native-nd module be skipped over for linking in this app?

At this moment it'll be hidden behind --dependency-check flag to make this process optional

Yep, just wondering if we'd be able to make use of it in our monorepos. :) It'd save us having to respecify shared native modules in each app's package.json so that they are 'seen' by the current autolinking process.

[TMisiukiewicz]

Hi @liamjones, sorry for late response - lots of stuff were going on because of RNEU conference lately. No changes are made to the autolinking mechanism itself - so this should work. I tested it with a single RN project and the package is not autolinked if added to react-native.config.js file.

[thymikee]

This is introducing new logic for determining whether this is a native dependency or not, and project can configure their own through react-native.config.js. Instead we should reuse existing mechanism so it's deterministic. Check how config calls platforms projectConfig:

cli/packages/cli-config/src/loadConfig.ts

Lines 43 to 46 in d695144

	: platformConfig.dependencyConfig(
	root,
	config.dependency.platforms[platform],
	);

and
Here's a reference implementation for Android:

cli/packages/cli-platform-android/src/config/index.ts

Lines 34 to 86 in d695144

	export function projectConfig(
	root: string,
	userConfig: AndroidProjectParams = {},
	): AndroidProjectConfig | null {
	const src = userConfig.sourceDir || findAndroidDir(root);
	if (!src) {
	return null;
	}
	const sourceDir = path.join(root, src);
	const appName = getAppName(sourceDir, userConfig.appName);
	const manifestPath = userConfig.manifestPath
	? path.join(sourceDir, userConfig.manifestPath)
	: findManifest(path.join(sourceDir, appName));
	const buildGradlePath = findBuildGradle(sourceDir, false);
	if (!manifestPath && !buildGradlePath) {
	return null;
	}
	const packageName =
	userConfig.packageName || getPackageName(manifestPath, buildGradlePath);
	if (!packageName) {
	throw new CLIError(
	`Package name not found in neither ${manifestPath} nor ${buildGradlePath}`,
	);
	}
	const applicationId = buildGradlePath
	? getApplicationId(buildGradlePath, packageName)
	: packageName;
	const mainActivity = getMainActivity(manifestPath || '');
	if (!mainActivity) {
	throw new CLIError(`Main activity not found in ${manifestPath}`);
	}
	return {
	sourceDir,
	appName,
	packageName,
	applicationId,
	mainActivity,
	dependencyConfiguration: userConfig.dependencyConfiguration,
	watchModeCommandParams: userConfig.watchModeCommandParams,
	unstable_reactLegacyComponentNames:
	userConfig.unstable_reactLegacyComponentNames,
	};
	}

[thymikee]

I don't get what this test verifies. What's react-native-safe-area-view changing? Are those all the keys available in filtered or only a few?

[TMisiukiewicz]

Removing check for '@react-navigation/stack' as its not relevant here. react-native-safe-area-view is a mocked dependency with native code, which is a peer dependency of @react-native/elements, and @react-native/elements is a dependency of @react-navigation/stack. This test proves that recursively collected packages dependent on peer deps with native code will be listed

[acoates-ms]

Building up the config is one of the things that causes the current CLI to feel sluggish on a number of commands when working in projects with larger numbers of dependencies. Making config do transitive dependency transversals will make the CLI even slower.

I know its opt in right now, but it feels like something you really don't want to be doing on every CLI command.

This check and fix logic feels like something that would be much more appropriate written as a doctor health check.

[TMisiukiewicz]

@acoates-ms yeah i agree, since npm and bun automatically install peer dependencies, the check itself is pretty fast. The biggest problem is yarn where it needs to install all the peer dependencies recursively first to find out which one of them contain native code. I assume yarn is a majority when it comes to package managers, so yeah that would be a bit overwhelming.
In general, I like the idea of moving it to a doctor, however for yarn users, it would still be more time consuming. On the other hand, doctor is not a command used every day, it should be fine if it takes a bit more time than now. It could play nicely with a menu available after displaying an output:

 › Press f to try to fix issues.
 › Press e to try to fix errors.
 › Press w to try to fix warnings.
 › Press Enter to exit.

we could print the missing packages and use w to eventually install them

Also it seems related to #1983

[github-actions]

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

[ikedm]

@TMisiukiewicz thanks for your continued push on this.

Are there any updates?

From what I can see there's no current way to get this working, correct? Do we know if ExpoCLI supports including transitive deps in autolinking?

[TMisiukiewicz]

hi @dalmendray, unfortunately there is no update about this one at this moment. I want to get back to this, but probably in a little bit different shape, once all platform-specific commands will be moved to the RN core.

I think Expo CLI autolinks transitive dependencies only for Expo Modules

[github-actions]

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

[github-actions]

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.