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

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

Closed
motiz88 opened this issue Sep 2, 2022 · 26 comments
Closed

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

motiz88 opened this issue Sep 2, 2022 · 26 comments
Labels
enhancement New feature or request feature: align-deps This is related to align-deps

Comments

@motiz88
Copy link

motiz88 commented Sep 2, 2022

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 ;)

@ghost ghost added the needs triage 🔍 label Sep 2, 2022
@tido64 tido64 added enhancement New feature or request feature: align-deps This is related to align-deps and removed needs triage 🔍 labels Sep 2, 2022
@tido64
Copy link
Member

tido64 commented Sep 2, 2022

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

@tido64
Copy link
Member

tido64 commented Sep 8, 2022

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
Copy link

I think this would be an amazing addition to the New Architecture migration story.
It has been asked previously by the community (see facebook/react-native-website#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
Copy link
Member

tido64 commented Sep 8, 2022

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
Copy link

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
Copy link
Member

kelset commented Sep 12, 2022

This conversation I think is very close to what the RNDirectory is trying to achieve through this: react-native-community/directory#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
Copy link
Member

tido64 commented Sep 12, 2022

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
Copy link

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
Copy link
Member

tido64 commented Sep 12, 2022

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
Copy link

huntie commented Sep 14, 2022

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

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

  • Preliminarily aligned on package.json over react-native.config.js as the location for package-defined options.
  • Are considering a root key (name TBC, e.g. react-native-config) to encapsulate these options.

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
Copy link
Author

motiz88 commented Sep 14, 2022

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

@tido64
Copy link
Member

tido64 commented Sep 14, 2022

@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:

  • 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?

@tido64
Copy link
Member

tido64 commented Sep 16, 2022

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

@tido64
Copy link
Member

tido64 commented Sep 22, 2022

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: th3rdwave/react-native-safe-area-context#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
Copy link

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
Copy link
Member

tido64 commented Sep 23, 2022

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
Copy link
Member

kelset commented Oct 11, 2022

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
Copy link

huntie commented Oct 11, 2022

@kelset Very close to publishing, will include the "exports" RFC link here when done: react-native-community/discussions-and-proposals#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
Copy link
Member

kelset commented Oct 11, 2022

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
Copy link
Member

kelset commented Dec 1, 2022

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 (nodejs/node#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:

  • 🔴 = no support for new arch at all
  • 🟡 = we can infer that it will probably work with the new arch based on . For example, we could use the same one that RNDirectory uses, based on the presence of the codegenConfig field in package.json
  • 🟢 = library has our custom-setting field set to true

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

@kelset kelset changed the title [Feature request] Use dep-check to help migrate apps to the New Architecture [Feature request] Use align-deps to help migrate apps to the New Architecture Dec 1, 2022
@huntie
Copy link

huntie commented Dec 1, 2022

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
Copy link
Member

kelset commented Dec 8, 2022

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 kelset changed the title [Feature request] Use align-deps to help migrate apps to the New Architecture Use align-deps to help migrate apps to the New Architecture Jan 3, 2023
@kelset kelset pinned this issue Jan 3, 2023
@kelset
Copy link
Member

kelset commented Jan 4, 2023

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
Copy link
Member

kelset commented Jan 25, 2023

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

@kelset
Copy link
Member

kelset commented Aug 7, 2023

headsup: we are still blocked on this waiting on the RFC to make progress: react-native-community/discussions-and-proposals#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
Copy link
Member

kelset commented Apr 4, 2024

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.

@kelset kelset closed this as completed Apr 4, 2024
@kelset kelset unpinned this issue Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature: align-deps This is related to align-deps
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants