microsoft / rnx-kit

Modern, scalable tools. Exceptional developer experience.
https://microsoft.github.io/rnx-kit/
MIT License
1.5k stars 96 forks source link

Use align-deps to help migrate apps to the New Architecture #1863

Closed motiz88 closed 5 months ago

motiz88 commented 2 years ago

For app maintainers to migrate to the New Architecture, all of their native dependencies must support the New Architecture as well. This isn't the same as being merely compatible with some React Native version 0.x, since (for the time being) the New Architecture is opt-in.

A problem that has come up is that it's not easy to know ahead of time, as an app maintainer, whether all of your dependencies have new versions available that are compatible with the New Architecture. In the worst case, you might sink time and effort into beginning an upgrade, only to get blocked halfway by some third-party library and have to abandon the effort for an unknown length of time.

The idea here would be to add information to align-deps's database about various common libraries' New Architecture readiness, such that (ideally) we can recommend align-deps as part of the official New Architecture migration guide for app maintainers. In doing this, we want to (1) increase the likelihood of apps migrating successfully on the first try, (2) create visibility in the ecosystem (and thus momentum and excitement) around popular libraries migrating to the New Architecture.

There are some questions around how we source this information reliably at scale (e.g. combine efforts with https://reactnative.directory/ ?), but in the short term leveraging align-deps's standalone database of packages seems like a good way to jumpstart this capability for relatively low effort.

NOTE: I'm only lightly familiar with align-deps and the New Architecture, so I've kept this super high-level and am not opinionated on the details.

cc @cortinico @tido64 @afoxman @huntie @cipolleschi


update by @kelset: changed dep-check to align-deps ;)

tido64 commented 2 years ago

We can probably use the list in https://github.com/reactwg/react-native-new-architecture/discussions/6 as a starting point.

tido64 commented 2 years ago

We can probably use the list in reactwg/react-native-new-architecture#6 as a starting point.

I have a draft PR with only these packages. The rest are null-ed out.

cortinico commented 2 years ago

I think this would be an amazing addition to the New Architecture migration story. It has been asked previously by the community (see https://github.com/facebook/react-native-website/issues/3275).

As for the technical side: we don't have a simple way to know if a library is New Architecture compatible. In general, relying on codegenConfig inside package.json is a good indicator. However there might be other libraries which are New Architecture compatible and not using the Codegen. We should think about an indicator that allows us to easily understand this (like another field in the package.json or so).

tido64 commented 2 years ago

Is codegenConfig always present in a fully migrated package? I extended the script we use to check for updated packages to include it:

Capability Name Version Latest New Arch Homepage
animation react-native-reanimated ^2.10.0 = https://github.com/software-mansion/react-native-reanimated#readme
base64 react-native-base64 ^0.2.1 = https://github.com/eranbo/react-native-base64#readme
checkbox @react-native-community/checkbox ^0.5.8 0.5.12 https://github.com/react-native-community/react-native-checkbox#readme
clipboard @react-native-clipboard/clipboard ^1.10.0 1.11.0 https://github.com/react-native-clipboard/clipboard#readme
datetime-picker @react-native-community/datetimepicker ^6.0.2 6.3.2 https://github.com/react-native-community/datetimepicker#readme
filesystem react-native-fs ^2.18.0 2.20.0 https://github.com/itinance/react-native-fs#readme
floating-action react-native-floating-action ^1.22.0 = https://github.com/santomegonzalo/react-native-floating-action#readme
gestures react-native-gesture-handler ^2.6.0 = Yes https://github.com/software-mansion/react-native-gesture-handler#readme
hooks @react-native-community/hooks ^2.8.0 2.8.1 https://github.com/react-native-community/hooks#readme
html react-native-render-html ^6.1.0 6.3.4 https://meliorence.github.io/react-native-render-html/
jest jest ^26.6.3 29.0.2 https://jestjs.io/
lazy-index @rnx-kit/react-native-lazy-index ^2.1.7 = https://github.com/microsoft/rnx-kit/tree/main/packages/react-native-lazy-index#readme
masked-view @react-native-masked-view/masked-view ^0.2.7 = https://github.com/react-native-masked-view/masked-view#readme
modal react-native-modal ^13.0.0 13.0.1 https://github.com/react-native-community/react-native-modal
navigation/native @react-navigation/native ^6.0.8 6.0.12 https://reactnavigation.org
navigation/stack @react-navigation/stack ^6.2.0 6.2.3 https://reactnavigation.org/docs/stack-navigator/
netinfo @react-native-community/netinfo ^9.0.0 9.3.0 https://github.com/react-native-netinfo/react-native-netinfo#readme
popover react-native-popover-view ^5.0.0 5.1.2 https://github.com/steffeydev/react-native-popover-view#readme
safe-area react-native-safe-area-context ^4.3.1 4.3.3 Yes https://github.com/th3rdwave/react-native-safe-area-context#readme
screens react-native-screens ^3.14.1 3.17.0 Yes https://github.com/software-mansion/react-native-screens#readme
shimmer react-native-shimmer ^0.5.0 0.6.0 https://github.com/oblador/react-native-shimmer
sqlite react-native-sqlite-storage ^6.0.1 = https://github.com/andpor/react-native-sqlite-storage
storage @react-native-async-storage/async-storage ^1.17.10 = https://github.com/react-native-async-storage/async-storage#readme
svg react-native-svg ^12.3.0 13.1.0 Yes https://github.com/react-native-community/react-native-svg
test-app react-native-test-app ^1.6.9 1.6.13 https://github.com/microsoft/react-native-test-app
webview react-native-webview ^11.23.0 11.23.1 https://github.com/react-native-webview/react-native-webview#readme

Reanimated isn't listed here because New Arch support is still in RC.

cortinico commented 2 years ago

Is codegenConfig always present in a fully migrated package? I extended the script we use to check for updated packages to include it:

As mentioned above, not always. There might be packages without codegenConfig which support the New Architecture. That being said, I think it's anyway a good indicator.

kelset commented 2 years ago

This conversation I think is very close to what the RNDirectory is trying to achieve through this: https://github.com/react-native-community/directory/pull/870

cc @simek

It'd be great to decide a standard way to communicate new arch support, and then "enforce it" by having both dep-check and directory leverage it as the official way. At that point we can communicate in all the 3 places (dep-check, RN docs website, RNDirectory) that doing "XXX" is the one way.

tido64 commented 2 years ago

I've been wondering if it makes more sense for each package to add a field that signals their new-arch readiness. It would allow cli to warn when autolinking, dep-check/directory to generate the compat table, and docs to be automatically updated.

Edit: The field does not need to be binary, it could signal compatibility, e.g. "ready" -> works fine as-is, "full" -> codegen and extra steps to ensure better experience.

cortinico commented 2 years ago

I've been wondering if it makes more sense for each package to add a field that signals their new-arch readiness.

Yup that's a good thing to add. Do you have any suggestion (I assume it's going to be stored inside the package.json or the react-native.config.js in some form).

tido64 commented 2 years ago

Yup that's a good thing to add. Do you have any suggestion (I assume it's going to be stored inside the package.json or the react-native.config.js in some form).

I think package.json would make the most sense since you can query it using npm instead of downloading the full package just to inspect a single file.

huntie commented 2 years ago

@cortinico @tido64 Small input on using package.json:

For an upcoming feature in Metro (exports support), we:

Therefore if we are looking to introduce new explicit key(s) for this feature, it might sit well grouped under this:

{
  "name": "my-package",
  "react-native-config": {
    "metro-exports-mode": "strict",
    "new-arch-support": "ready"
  }
}

Edit: Renamed overloaded react-native key — temporary name for now, suggestions welcome.

motiz88 commented 2 years ago

@huntie We can't use react-native, specifically - that key is already used as a React Native-specific variant of browser.

tido64 commented 2 years ago

@huntie: Yes, I think adding a new field like you described makes the most sense. It would allow for future expansions.

If we're all agreeing on this approach, what would be the next steps? In my mind, I'm thinking we should at least start with:

Unrelated: Would it also make sense to move codegenConfig into react-native-config?

tido64 commented 2 years ago

cc @brentvatne in case you have better ideas or feedback on this.

tido64 commented 1 year ago

Another interesting bit I discovered today: Even with codegenConfig present, it's not a guarantee that the library supports New Arch + autolinking. I've spent days (on separate occasions) trying to get to the bottom of this: https://github.com/th3rdwave/react-native-safe-area-context/issues/303

Which brings me back to this:

If we're all agreeing on this approach, what would be the next steps? In my mind, I'm thinking we should at least start with:

  • [ ] Communicating this "requirement" to the community (Meta?)
  • [ ] Update rn-cli to scan for this field if new arch is enabled, and warn about potentially unsupported modules and teach users how to get rid of the warning (Callstack?)
  • [ ] Add the dep-check profile with new arch only packages to help users upgrade their projects, periodically updated using a script that scans this new field (Microsoft)
  • [ ] rn-directory to update their scripts (Expo?)

Unrelated: Would it also make sense to move codegenConfig into react-native-config?

I'd love to hear what people think should be the way forward.

cortinico commented 1 year ago

Even with codegenConfig present, it's not a guarantee that the library supports New Arch + autolinking

That's what I mentioned before. We need an explicit field to mention that the New Architecture is supported.

Is this the preferred approach then? nit: react-native-config vs codegenConfig are mixing camel case with kebab case, not great.

  "react-native-config": {
    "metro-exports-mode": "strict",
    "new-arch-support": "ready"
  }

Unrelated: Would it also make sense to move codegenConfig into react-native-config?

Potentially yes. Yet please note that both Gradle and CocoaPods are parsing the package.json. Moving things inside a react-native.config.js which is not a JSON file, will make it hard to parse it.

Also speaking with @cipolleschi, we were brainstorming about having a single variable "new-arch-enabled": "true" for Applications to enable/disable the new arch (instead of having separate configs for iOS and Android). If we do so, it would be great to use the same key/value for Libraries and Apps.

tido64 commented 1 year ago

Is this the preferred approach then? nit: react-native-config vs codegenConfig are mixing camel case with kebab case, not great.

  "react-native-config": {
    "metro-exports-mode": "strict",
    "new-arch-support": "ready"
  }

I'm not a fan of mixing cases either. We should probably use camelCase (i.e. reactNativeConfig) since that seems to be the style used in package.json

Unrelated: Would it also make sense to move codegenConfig into react-native-config?

Potentially yes. Yet please note that both Gradle and CocoaPods are parsing the package.json. Moving things inside a react-native.config.js which is not a JSON file, will make it hard to parse it.

I meant the react-native-config key (or reactNativeConfig), not the file. Sorry for the confusion 😛

kelset commented 1 year ago

gentle nudge to the thread: @huntie how is the work on the export feature? Have you decided on the package.json field name? react-native-config or reactNativeConfig?

huntie commented 1 year ago

@kelset Very close to publishing, will include the "exports" RFC link here when done: https://github.com/react-native-community/discussions-and-proposals/pull/534.

Standardising on camel case throughout makes sense. The RFC will contain a new proposed key for package.json, "reactNative":

  "reactNative": {
    "metroExportsMode": "strict",
    "newArchSupport": "ready"
  }

With this casing change, we can end up keeping a simple and wide name to group these React Native config options and/or future metadata. The downside here is the chance of confusion with the existing key.

kelset commented 1 year ago

I guess we can then bring over the conversation about the name of the config to the RFC once it's up. Not a big fun of having "react-native" and "reactNative" be two totally different things.

kelset commented 1 year ago

Hey folks; just wanted to follow up here now that thanks to @huntie and the rest of the Metro team it seems that the exports support is going ahead and the definition of "react-native" has been accepted in node itself (https://github.com/nodejs/node/pull/45367).

If I'm understanding correctly, it means that "react-native" is now officially used for that - and now we need to create a different name for section in the package.json along the lines of:

  "react-native-config": {
      "custom-setting": "entry",
      ...
  },
  "dependencies": {
     // etc etc

Right? And not only that, we need to decide what to name the field for identifying react-native's new arch support, right?


On top of this, I wanted to also check in into an idea that the developers from InfiniteRed brought to us for indicating via align-deps which libs support the new arch or not. Basically, they were proposing something a bit more in-depth than just a binary yes/no, but something more like a semaphore:

What do you think? I think this might help and also we could potentially "mirror" this into the RNDirectory website too.

huntie commented 1 year ago

it means that "react-native" is now officially used for that

Clarifying: The "react-native" root field was originally defined and used by us (Metro, Webpack, others) as an alternative for the "browser" field spec/"main" field, and is already in wide use (with package exports this concept is being replaced with an exports condition of the same name — however we will not be pushing packages to migrate and remove this root field).

The impact is the same, however (and the Metro exports spec no longer depends on this decision)! We need a different root field name to "react-native" to avoid conflict, such as "react-native-config" (as previously suggested, likely good to go) or "reactNative" (potentially too indistinct).

kelset commented 1 year ago

Totally, thanks for the clarification @huntie - we did a meeting earlier this week with @motiz88 and Blake and other Meta folks, I'm guessing the next step is to understand how we want to go ahead with this. My current latest understanding is that internally during planning you will decide how you want to go ahead with this in H123 and then we can create some actionables.

kelset commented 1 year ago

Reviving this thread one more time, since now we should all be approaching clarity over how the next few months would look like.

I feel that the next step would be to get a few folks in a room and make sure that we can reach an agreement on the final "spec" for this new react-native-config section of the package.json. I'm a bit foggy but I am pretty sure that Expo and the RN CLI already have some variations of local configurations (IIRC, using app.json and react-native.config.js) so I reckon that whatever shape we end up agreeing on, we should realign all these "side configurations" into a main, unified and spec'd one.

I'd like to get a few folks in a room to discuss details and after that, setup a "final" RFC for how to approach so that we can coordinate the needed work. As attendees, I'm thinking some representatives from CLI, Expo, Metro and rnx-kit, keeping the number of people below 10.

kelset commented 1 year ago

quick update: we are now working on said configuration standard, read more here: https://github.com/react-native-community/discussions-and-proposals/pull/588

kelset commented 1 year ago

headsup: we are still blocked on this waiting on the RFC to make progress: https://github.com/react-native-community/discussions-and-proposals/pull/588

AFAIK it will be actually get closed in favour of a 3.0 rewrite as a separate PR; I'll post an update once it's there.

kelset commented 5 months ago

An update on this: since the last time we talked about this, there have been a couple of relevant updates:

1) libraries have started supporting new arch & bridgeless. So, indirectly, just by keeping up and having profiles for 0.73 and 0.74 for align-deps, we do have somewhat a level of support for developers using align-deps in making sure that they are using versions of libs that do have newarch/bridgeless support.

2) @aleqsio made a small handy tool that checks a repository package.json against the (manually maintained) gSheet linked in the discussion above. While not ideal nor programmatic (since it's source of truth is a man-mantained gSheet), me and Tommy think it does a pretty great job.

So devs at this point in time have a pretty decent level of support in migrating their apps to the new arch.

Because of this, we think we can close this off. Let us know if you have other thoughts on the matter.