🦋 Changeset detected

Latest commit: 220ba63

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This is awesome 🙏 Let's move it as a separate workspace (packages/default-storage-expo-plugin) and we'll do npm releases from there. I understand then expo users will have to install this plugin via npm to use it, correct?

Hi @krizzu, thanks for the kind words <3

Happy to move the code to a separate workspace, but I just wanted to note that typically Expo plugins are included with the native module package itself (see Expo Contacts or Expo Calendar) - an example from one of my projects looks like:

{
  "expo": {
    "plugins": [
      [
        "expo-build-properties",
        {
          "android": {
            "kotlinVersion": "1.9.23"
          }
        }
      ],
      "expo-router",
      [
        "expo-calendar",
        {
          "calendarPermission": "$(PRODUCT_NAME) needs to access your calendar to sync your events and performances."
        }
      ],
      [
        "expo-contacts",
        {
          "contactsPermission": "Allow $(PRODUCT_NAME) to access your contacts."
        }
      ]
    ]
  }
}

My suggestion would be to

1. Move the plugin source to packages/default-storage-expo-plugin
2. Add packages/default-storage-expo-plugin as a direct dependency of @react-native-async-storage/async-storage
3. Add app.plugin.js to @react-native-async-storage/async-storage, referencing packages/default-storage-expo-plugin

typically Expo plugins are included with the native module package itself
So that in app.config.js you just type in @react-native-async-storage/async-storage, because plugin is already bundled there, right?

Async storage is used outside expo as well, so I'd rather not add anything expo specific, unless necessary. Seems like an extra package to install and configured in app.config.js is not a big deal (especially having documentation describing process).

wdyt @tido64 ?

Async storage is used outside expo as well, so I'd rather not add anything expo specific, unless necessary.

That does make sense - my rationale for it being there is the recommendation of Expo as the framework for React Native apps (paraphrasing the React Native devs here).

Additionally, Expo config plugins can also be used for out-of-tree RN platforms; see https://github.com/byCedric/custom-prebuild-example.

From what I've seen, the convention is to include the plugin in the same package.

However, looking at the plugin implementation, that would mean that we have to add a direct dependency on @expo/config-plugins. The plugin is currently importing it, but have not declared it as a dependency. This will cause issues because now we're dependent on very specific hoisting behaviours.

That said, I don't like that fact that we're now forcing all users of AsyncStorage to download @expo/config-plugins regardless of whether they'll use it. This PR adds 3000+ lines to yarn.lock. So I think I'm in favour of having a separate package + install.

I'm also not a fan of the option name, useNextStorage. This only applies to Android and it's not obvious by just looking at the name. Can we add platform prefix i.e., android_?

Thanks for the comments @tido64, I've updated the PR accordingly.

However, looking at the plugin implementation, that would mean that we have to add a direct dependency on @expo/config-plugins. The plugin is currently importing it, but have not declared it as a dependency. This will cause issues because now we're dependent on very specific hoisting behaviours.

My understanding is that @expo/config-plugin is a dependency of Expo now, so when a config plugin is installed into an Expo project, it will use the project-local version of @expo/config-plugin. Looking at the config plugins here seems to confirm this.

That said, I don't like that fact that we're now forcing all users of AsyncStorage to download @expo/config-plugins regardless of whether they'll use it. This PR adds 3000+ lines to yarn.lock.

This would only be true if @expo/config-plugin was a direct dependency of the plugin.

I'm also not a fan of the option name, useNextStorage. This only applies to Android and it's not obvious by just looking at the name. Can we add platform prefix i.e., android_?

I used the name as described in the documentation (which already indicates that it's only for Android). Instead of prefixing it, I've nested it since there are other configurable options that I haven't added support for.

My understanding is that @expo/config-plugin is a dependency of Expo now, so when a config plugin is installed into an Expo project, it will use the project-local version of @expo/config-plugin. Looking at the config plugins here seems to confirm this.

That part is fine. However, dependencies may not be installed in such a way that the plugin will be able to find Expo. In a standalone repo with a single app, all dependencies are installed under node_modules and everything is mostly fine. But in a monorepo, things get a little more complicated. For instance, dependencies could be installed like this:

root
├── packages
│   └── my-app
│       └── node_modules
│           ├── @react-native-async-storage/async-storage
│           └── expo
└── node_modules

In this specific case, the plugin will be able to find Expo. But this isn't always the case. @react-native-async-storage/async-storage could be hoisted like this:

root
├── packages
│   └── my-app
│       └── node_modules
│           └── expo
└── node_modules
    └── @react-native-async-storage/async-storage

In this case, the plugin will not be able to find Expo. This is why we cannot rely on implicit dependencies. We always declare our dependencies.

This would only be true if @expo/config-plugin was a direct dependency of the plugin.

Since it looks like expo re-exports @expo/config-plugins, another option is to make expo an optional dependency:

  "peerDependencies": {
    "expo": ">=51"
  },
  "peerDependenciesMeta": {
    "expo": {
      "optional": true
    }
  },

This should allow us to keep the footprint to a minimum.

@hassankhan thanks, appreciate the changed 🙏

I think we missed the other configuration bits from the next storage, such as kotlin, ksp and room versions. Can you add this in to config as well?