Depending on Expo modules in other packages #13963

brentvatne · 2021-08-07T00:52:21Z

brentvatne
Aug 7, 2021
Maintainer

It's common to want to depend on Expo modules from other npm packages. While certainly not the only case where this applies, you will find this in error reporting libraries (eg: bugsnag, sentry) and analytics libraries (eg: segment, firebase), because they often want to automatically add some context to events, such as the device type, operating system version, app version, etc.

React Native doesn't expose any core primitives to access this information, so these libraries need to depend on either their own native code (traditionally not supported in managed apps, although it is supported now with custom dev clients) or another native library that exposes an interface to the desired data. For example: expo-device, expo-application, expo-constants, expo-updates. So, these libraries add dependencies on packages like expo-device.

Not really though, jk.

Problems with depending on Expo SDK packages

eg: expo-device@~3.3.0 works with Expo SDK 42 / react-native-unimodules 0.14.6

In Expo Go, this is a hard requirement because you don't control the native runtime. In EAS Build, there is some flexibility, but Expo SDK packages are built alongside react-native-unimodules (soon to be expo-modules-core) and there may be some compatibility issues if the versions are not aligned.

The result is that you will need to have releases of your package for every Expo SDK version to ensure the dependencies line up properly with those compatible with specific Expo SDKs. You'll need to update it each time a new Expo SDK is released. There also is not yet a way for third party packages to hook into expo install and expo upgrade to ensure the correct version is installed for the project (this is something that we do intend to support in the future). But overall, depending on specific package versions is not great.

Subproblem: npm is not designed to handle native dependencies

To further complicate things, npm does not do so well with the constraints imposed by native dependencies. With JavaScript dependencies, it's often okay (not always, sometimes this is problematic due to package level statefulness) to just install whatever version of a package your library needs, and if another library needs another version of that package, that's not a problem. Each one can import the version they need, everyone is happy. This is problematic for native dependencies in React Native because you can only link one module at a time, and then the JS code for all instances of that package in your node_modules will run against that same version of the native interface, which can result in hard crashes on app launch (eg: if it's a view manager), obscure errors (eg: if interface changed in a way that doesn't crash but returns different results), and all sorts of other problems.

What about peer dependencies?

Cool, so maybe we can just avoid this by using peer dependencies? We can say that this library requires a peer dependency "expo-device": "*" or "expo-device": "~3.3.0 || ~4.0.0" and then the user will be warned that they need to install this library, right? Right?

No. The behavior of peer dependencies changed in npm 7, so now it will be automatically installed if needed, rather than just warn.

If we depend on "expo-device": "*" then npm 7 will install the latest version of expo-device, rather than the version used by the project's Expo SDK (because npm doesn't and shouldn't know anything about Expo SDK versions). If this installed an incompatible version of expo-device, users would be warned in expo start or expo doctor:

╭─~/code/ppa ‹master›
╰─$ expo start
Starting project at /Users/brentvatne/code/ppa
Developer tools running on http://localhost:19002
Some dependencies are incompatible with the installed expo package version:
 - expo-device - expected version: ~3.2.0 - actual version installed: 3.3.0
Your project may not work correctly until you install the correct versions of the packages.
To install the correct versions of these packages, please run: expo install [package-name ...]
# ... continues to start as usual, the above is just a warning

This is not the worst result, and it is what is done in sentry-expo, but it could still result in users having broken apps if they ignore these warnings. This can be mitigated by advising users to install the peer dependencies as dependencies along with the package installation instructions:

Now this ensures the correct version will be used and they will be upgraded with expo upgrade to continue working in the future.

If we use "expo-device": "~3.3.0 || ~4.0.0" for our peer dependency we would possibly to update the package for every new SDK version, or whenever a version is released that has a breaking change that requires a version bump that falls out of the compatible range, even if the functionality depended on does not break. If we use "expo-device": "*", we may end up depending on an API that has changed or no longer exists. Both of these appear to be legitimate options depending on the preferences of the library maintainer, a decision that they can make based on the expected failure mode.

What about optional dependencies?

The npm idea of "optional dependencies" doesn't work here, but using try/catch around require allows us to conditionally use a library if it is available.

// Device.js
let Device;
try {
  Device = require('expo-device');
} catch {
  // no device available
  console.warn('If it is very important, possibly warn?');
}

export function getBrand() {
  return Device ? Device.brand : 'unknown';
}

// etc..

// elsewhere
import * as Device from './Device';

Device.getBrand();

Library authors can then include the package in their devDependencies but leave it out of peer and regular dependencies.

@LinusU is not a big fan of this approach:

I feel that doing an import of a package not declared in your dependencies (peer or not) can easily lead to problems. It basically takes away the dependency handling from npm and places all the burden on the end user instead.

What about dependency injection?

Libraries like redux-persist have solved similar problems by requiring that the developer pass in the API in the library configuration:

import AsyncStorage from '@react-native-community/async-storage';

const persistConfig = {
  //...
  storage: AsyncStorage
}

This is another possible solution in some cases. It has the benefit of being very explicit, and also gives users the ability to swap in their own implementations, provided they conform to the expected interface. The downside is that it no longer "Just Works".

This thread is intended to be an open discussion about this problem, please share any thoughts or opinions.

LinusU · 2021-08-07T13:49:47Z

LinusU
Aug 7, 2021

Just to expand on the optional dependencies part. There actually is a way to declare optional peer dependencies in npm, using the peerDependenciesMeta property in the package.json.

If the packages is actually optional (that is, the package will work without the dependency) I don't think that there is problem with doing a try/catch import of it. But in that case I think that there is a benefit of specifying that as an optional peer dependency so that npm can still now about the relation so that it tries to move the dependencies to the right place in the dependency graph so that it can be imported.

Now, in the case of Sentry/Bugsnag, I don't think that optional dependencies is all that great in this case. It's just pushing the error further down, when all metadata shows up as "unknown" in your error reports. Whilst at the same time there wasn't any technical reason for why expo-device couldn't be installed. I personally would rather get an error when building the app, with a one liner to fix it (expo add expo-device), rather than having missing metadata later...

I think optional dependencies is better suited when some functionally is not available on a specific platform, which is just a subset of the platform being supported by that package.

That being said, I was not aware of the warnings that expo prints. I think that this is great, but I would personally prefer it if they were actual errors (which could have a flag to ignore them and turn them into warnings).

If we have that, and then make sure that the ecosystem is using peer dependencies, I think that we would have largely solved this issue.

To reiterate from the thread that we had which sparked this discussion, I think that a huge drawback with not declaring any dependencies and then importing them anyways is that it's very easy to forget why dependencies was added, and it's easy to accidentally remove them in the future. It basically moves the responsibility of keeping track of dependencies from npm to the end user.

As an example, you have a project where you are already using expo-device for a feature in your app. You now install Sentry which also tells you to run expo add expo-device. Since expo-device is already in your project that doesn't really do anything, but at this point everything works great.

Now you are doing changes to your app and you remove your code that used expo-device. Since this was your last import of that package you also remove expo-device. But now you will either have "unknown" for all your metadata at Sentry, or Sentry will fail to load at runtime without any what to statically analyse to know this (since npm, or any other tool, doesn't now that expo-sentry uses expo-device).

In fact, if you fix this by adding back expo-device to your dependencies but you aren't using it anywhere, then tools like npm-check will complain and ask you to remove the dependency.

The more I think about this I think that the best approach is to use peer dependencies in the libraries, and let expo error out if you have the wrong version of the package anywhere in your dependency graph.

The reason for using peer dependencies instead of normal dependencies in the libraries is because there can only be exactly one version of this packages at the same time, and using a normal dependency will allow npm to add that dependency deeper in the graph to satisfy different requirements...

This was just a quick brain dump that became quite long 😅

I'm very happy to continue the discussion and very open to all arguments for different solutions!

3 replies

imjoehaines Aug 10, 2021

There also is not yet a way for third party packages to hook into expo install and expo upgrade to ensure the correct version is installed for the project (this is something that we do intend to support in the future).

This sounds like a really good solution, though I guess would still require some manual work from packages to note which SDK versions each release is compatible with (?) That still solves a lot of the current issues though, so I'd be excited to see this happen

What about optional dependencies?

As @LinusU said, I think losing so much useful metadata would be a deal breaker for us. Our Expo delivery plugin also relies heavily on expo-file-system, so it's not really possible for us to make that an optional dependency or some users would have an install of Bugsnag that couldn't deliver events

The more I think about this I think that the best approach is to use peer dependencies in the libraries, and let expo error out if you have the wrong version of the package anywhere in your dependency graph.

One problem with this is that you still need to keep the peer dependency version exactly in sync with Expo or you can still end up with mismatched versions. That's not necessarily the end of the world as it's the same as the current situation though

Another problem is that some people will have to manually install the peer dependencies if they haven't updated to NPM v7. I'm also not sure if other package managers plan to (or already do) install peer dependencies by default. I know Yarn doesn't so it wouldn't be uncommon for users to have to manually install them

The reason for using peer dependencies instead of normal dependencies in the libraries is because there can only be exactly one version of this packages at the same time, and using a normal dependency will allow npm to add that dependency deeper in the graph to satisfy different requirements...

This is a really good point though — we've had issues in the past due to users ending up with multiple copies of the same Bugsnag package and peer dependencies fix it

I think we would prefer sticking with regular dependencies just so it's not possible to be missing packages that are required at runtime, but using peer dependencies might be the best option in other packages where dependencies could be made optional

A solution I was mulling over the weekend is to version @bugsnag/expo according to the Expo SDK version it supports, e.g. @bugsnag/expo v42.0.0 supports Expo SDK 42

Using a major version stops users from accidentally updating to an incompatible version and makes it immediately obvious if there's no support for the latest SDK. It does still require manual work with each Expo release, but that's no worse than the current solution

The other downside is requiring users manually update the version when they update Expo, but hopefully that should be fairly clear from the version scheme itself. If we could hook into expo upgrade that also solves this problem (depending what that hook allows us to do, of course)

It doesn't really solve the core issue of keeping native dependencies up to date, but should prevent users from trying to use an incompatible version of a package with the latest Expo SDK. Hopefully it'd be clear that using @bugsnag/expo v41 with Expo SDK 42 wouldn't work from the mismatched version numbers alone

I don't think it's compelling enough to switch @bugsnag/expo's versioning at this point, but might be a useful option for new packages to consider

Even if we do eliminate all the manual work involved in supporting a new SDK version, we'd still be doing manual testing to ensure everything still works and to catch any new deprecations. In theory it should be possible to automate a lot of the dependency updates since Expo already has a JSON file with the versions it uses, but I don't think we could totally eliminate all of the work we do to support a new SDK version

LinusU Aug 10, 2021

The more I think about this I think that the best approach is to use peer dependencies in the libraries, and let expo error out if you have the wrong version of the package anywhere in your dependency graph.

One problem with this is that you still need to keep the peer dependency version exactly in sync with Expo or you can still end up with mismatched versions. That's not necessarily the end of the world as it's the same as the current situation though

One benefit over the current status is that you can make your package work with many versions at runtime and then have a broad peer decency, e.g. "expo-file-system": "~9.3.0 || ~11.1.3" to support both SDK 40 and 42.

With the approach that Bugsnag Expo is using right now, minor version bumps of the package requires new versions of the Expo SDK which is a very breaking change...

Another problem is that some people will have to manually install the peer dependencies if they haven't updated to NPM v7. I'm also not sure if other package managers plan to (or already do) install peer dependencies by default. I know Yarn doesn't so it wouldn't be uncommon for users to have to manually install them

Npm <=6 prints a warning any time you run npm install, and I'm quite sure Yarn does that as well 🤔

I think we would prefer sticking with regular dependencies just so it's not possible to be missing packages that are required at runtime, but using peer dependencies might be the best option in other packages where dependencies could be made optional

The problem with this approach is exactly what happened in bugsnag/bugsnag-js#1483. I ended up with duplicate packages of expo-device, etc. deep down in my dependency graph. This error only manifested itself when I tried to build the app, and it's also possible that the problems would only be visible at runtime. I don't think that this is the best approach...

imjoehaines Aug 24, 2021

One problem with this is that you still need to keep the peer dependency version exactly in sync with Expo or you can still end up with mismatched versions. That's not necessarily the end of the world as it's the same as the current situation though

One benefit over the current status is that you can make your package work with many versions at runtime and then have a broad peer decency, e.g. "expo-file-system": "~9.3.0 || ~11.1.3" to support both SDK 40 and 42.

That has the problem in the OP though:

If we depend on "expo-device": "*" then npm 7 will install the latest version of expo-device, rather than the version used by the project's Expo SDK (because npm doesn't and shouldn't know anything about Expo SDK versions)

So if you're using SDK 40 you need expo-file-system ~9.3.0, but NPM will pull down ~11.1.3 as it doesn't know about SDK versions, which will be incompatible

If we're strict with the peer dependency version then I don't think it's any different to using real dependencies as we'd still need to update them with every new SDK version

Another problem is that some people will have to manually install the peer dependencies if they haven't updated to NPM v7. I'm also not sure if other package managers plan to (or already do) install peer dependencies by default. I know Yarn doesn't so it wouldn't be uncommon for users to have to manually install them

Npm <=6 prints a warning any time you run npm install, and I'm quite sure Yarn does that as well 🤔

In my experience no one looks at those warnings 😅 I've worked on many projects that have peer dependency warnings but work fine; I don't think users will action them automatically

I think we would prefer sticking with regular dependencies just so it's not possible to be missing packages that are required at runtime, but using peer dependencies might be the best option in other packages where dependencies could be made optional

The problem with this approach is exactly what happened in bugsnag/bugsnag-js#1483. I ended up with duplicate packages of expo-device, etc. deep down in my dependency graph. This error only manifested itself when I tried to build the app, and it's also possible that the problems would only be visible at runtime. I don't think that this is the best approach...

You'd still be able to end up in a very similar situation with peer dependencies though; either through using an older SDK version, which could install a newer version of a dependency or by using NPM 6/Yarn/etc… and not getting the dependency at all

I think realistically any solution will need to depend on knowledge of SDK and package versions, which could be through the Expo CLI or custom tooling (e.g. bugsnag-expo-cli already kind of does this, but only to cap the @bugsnag/expo version if you run it on an older SDK)

Navigation Menu

Repository navigation

Depending on Expo modules in other packages #13963

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Replies: 1 comment 3 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Depending on Expo modules in other packages #13963

Uh oh!

Uh oh!

brentvatne Aug 7, 2021 Maintainer

Problems with depending on Expo SDK packages

Subproblem: npm is not designed to handle native dependencies

What about peer dependencies?

What about optional dependencies?

What about dependency injection?

Replies: 1 comment · 3 replies

Uh oh!

LinusU Aug 7, 2021

Uh oh!

imjoehaines Aug 10, 2021

Uh oh!

LinusU Aug 10, 2021

Uh oh!

imjoehaines Aug 24, 2021

brentvatne
Aug 7, 2021
Maintainer

Replies: 1 comment 3 replies

LinusU
Aug 7, 2021