Skip to content
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.

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

RFC: Simplify workflow for native dependencies for libraries #870

Open
satya164 opened this issue Nov 27, 2019 · 11 comments
Open

RFC: Simplify workflow for native dependencies for libraries #870

satya164 opened this issue Nov 27, 2019 · 11 comments
Labels
feature request no-stale-bot This issue cannot be marked as stale by stale bot

Comments

@satya164
Copy link
Member

satya164 commented Nov 27, 2019

The problem

Currently the CLI looks for packages in user's dependencies and links them if they contain native code that needs to be linked. Which works great for apps. But there's no way for libraries to specify any libraries they depend on that contain native code.

The current approach is to use peer dependencies to specify them. However, due to React Native core not including things like gesture handler, reanimated, and due to lean core extracting built-in modules, it becomes cumbersome for the user to install tons of dependencies to use a single library.

The workflow for peer dependencies in npm/yarn isn't really great and the workflow is confusing.

Example scenario: Let's consider @react-navigation/stack as an example. It uses the following libraries which contain native code:

  • @react-native-community/masked-view
  • react-native-gesture-handler
  • react-native-reanimated
  • react-native-screens
  • react-native-safe-area-context

So, to use a single navigator, the user needs to install all of these packages manually into their project. It's discouraging and complicated for the user to have to install all these dependencies for a single package. Now instead of the library being charge of these dependencies, users have to make sure to install the correct version of these, and keep them updated themselves even if they don't use them.

What would be ideal is that the user should only need to install @react-navigation/stack and everything else should be taken care of automatically. This RFC is for discussing possible approaches and finding a good solution.

Possible solution

To reduce this overhead, in the library's package, we could define which native modules we depend on, let's say, in a new key dependencies (or the name you prefer) in react-native.config.js:

module.exports = {
  dependencies: [
    '@react-native-community/masked-view',
    'react-native-gesture-handler',
    'react-native-reanimated',
    'react-native-screens',
    'react-native-safe-area-context',
  ],
};

This way, the CLI knows that the library depends on these other libraries that need to be linked. Now, we could approach the solution in multiple ways:

1. Prompt the user to install these deps

The CLI could detect if the user already has these dependencies, and if not, show a selection list for installing them, then upon selection, install the correct versions according to the peer dependencies inside the library's package.json.

Optionally, CLI could also detect if in future the peer deps specify a different version and ask the user to upgrade to those versions as well.

2. Use regular deps and rely on flattening

Instead of peer dependencies, libraries could use regular dependencies for these packages instead. Then npm/yarn would flatten to single version if they are semver compatible.

For regular dependencies, the CLI would now need to look for libraries to link in nested node_modules as well (only for modules that have this new dependencies option).

If the CLI finds multiple versions installed, it would prompt the user to select which version to use. Then we'd need to force this version:

  1. Use resolutions options in yarn, overrides in npm
  2. Configure metro to always resolve to the selected version (how?)

Do you have any other ideas?

@thymikee
Copy link
Member

Thanks for sending the RFC! Transitive deps indeed are a limitation we currently have with linking. Ideally we'd like to perform such discovery automatically. The biggest challenge I see is packages with different versions, when a dependency would add native deps to its own deps and mangling with all the prompts, choices, storing all of that – gets messy quickly.

That's why I'm leaning towards the first option: "Prompt the user to install these deps". It should be relatively uncomplicated to implement and would provide better DX than we have currently. The libs would still need to declare peer dependencies though – which makes sense IMHO.

I'm really looking forward to have this tackled, so any help is greatly appreciated.

@brentvatne
Copy link
Contributor

brentvatne commented Nov 27, 2019

I agree that option 1 seems better but I'm hesitant to overload peerDependencies.. future changes to npm may create problems for us if we do. I can't find tweet at the moment but I saw a mention from a npm employee of possibly installing peerDependencies automatically again. Behavior for warnings in peerDependencies is already is different between npm and yarn, and semver validation in peerDependencies is different than with dependencies. In general I prefer to avoid this field entirely.

I think we should add another field to package.json or another config file (perhapsreact-native.config.js as suggested in RFC) that can indicate not just the required dependencies but which ones should be linked.

Another option is to have autolinking just link transitive dependencies without being told explicitly that they need to be linked. This is how the autolinking works for Expo - each native lib has a config file and we do a glob of node_modules to see which libraries need to be linked. If there are multiple versions of a library then we pick the latest version and warn the user -- we could go further and analyze the dependencies to indicate where the conflict arises and what could possibly be done to resolve it. Then we can just use dependencies. If there is a react-native install command similar to expo install then we can add validations before installing to warn people if they are going to install a version with conflicting dependencies.

daily-autobot pushed a commit to daily-co/react-native-daily-js-playground that referenced this issue Oct 14, 2020
…dencies with native code as `peerDependencies` rather than `dependencies`, which let this playground app make use of autolinking. See react-native-community/cli#870 for background
@whung1
Copy link

whung1 commented Apr 9, 2021

Hi all,

Was wondering if I could submit a PR using the methodology using a new methodology where react-native.config.js simply has

module.exports = {
  includesNativeDependencies: true
};

(perhaps having this flag in package.json is a better place. not sure on best practice here).

Then I will modify the code findDependencies.ts to check for this flag, and if true, additionally recurse down the package.json of the marked library for it's dependencies as well.

I have tested a version of this code locally and it added transitive dependencies fine with autolinking on Android, but I'm not sure on any possible other issues that may occur with manual links or iOS.

I think it should not have version issues due to the versions being in that package.json, but I have not tested this aspect.

Thanks.

@AlenToma
Copy link

AlenToma commented Jul 1, 2022

@whung1 The configuration for this should be up to the library.

I mean if I create a library and I am using external native libraries, then those library should be autolinked automatically right?

@github-actions
Copy link

There hasn't been any activity on this issue 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 github-actions bot added the stale label Nov 29, 2022
@thymikee
Copy link
Member

not stale

@github-actions github-actions bot removed the stale label Nov 30, 2022
@sesm
Copy link

sesm commented Dec 3, 2022

It would be great if current behaviour (not auto-linking transient native deps, advice for non-expo library authors to add peerDepencies) is explicitly documented somewhere, for example here: https://github.com/react-native-community/cli/blob/main/docs/autolinking.md
I don't like that when I need to explain current behaviour to people, I have to provide 2 github issues from 2 years ago as my source of truth.

@satya164
Copy link
Member Author

satya164 commented Dec 3, 2022

@sesm you can link them here: https://github.com/callstack/react-native-builder-bob#how-do-i-add-a-react-native-library-containing-native-code-as-a-dependency-in-my-library

@TMisiukiewicz TMisiukiewicz added the no-stale-bot This issue cannot be marked as stale by stale bot label Dec 6, 2022
@reylen
Copy link

reylen commented Mar 8, 2023

create file react-native.config.js in your project root (not your library), and then added like this:

module.exports = {

    dependencies: {
        '@react-native-community/masked-view': {},
        'react-native-gesture-handler': {},
        'react-native-reanimated': {},
        'react-native-screens': {},
        'react-native-safe-area-context':{},
    }
};

while pod install, then all the native modules will auto link to your project, it's useable for ios and android.
OR edit file react-native.config.js like this:

const myDependency = require("my-library/package.json")
 function format() {
    let _dependency = {};
    for (let myDependencyKey in myDependency.dependencies) {
        _dependency[myDependencyKey] = {};
    }
    return _dependency;
}
module.exports = {
    dependencies: format(),
};

@meypod
Copy link

meypod commented Jul 4, 2023

#1347 is probably solved by this as well (mentioning it for future devs, because it took me some time to land on this issue)

@meypod
Copy link

meypod commented Jul 5, 2023

@reylen's solution did not work for me, so I did the following:

const path = require('path');
const loadConfig =
  require('@react-native-community/cli-config/build/loadConfig').default;

const config = loadConfig(
  path.dirname(require.resolve('YourPackage/package.json')),
);

const dependencies = {};
Object.entries(config.dependencies).forEach(([k, v]) => {
  dependencies[k] = v;
  delete v.name;
});

module.exports = {
  dependencies,
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request no-stale-bot This issue cannot be marked as stale by stale bot
Projects
None yet
Development

No branches or pull requests

9 participants