Fix new architecture detection #1535
Conversation
There was a problem hiding this comment.
Hello @nympheastudio, thanks for bringing up an issue and attempting to address it! 👍
In would be nice if you can provide multiple examples of incorrect packages marking, which would help us to paint more broad picture of New Architecture support issues you are trying to address.
If this more a case of one, or up to few libraries which are mismatch, we probably should just alter their entries instead, and set newArchitecture: false if needed.
After diving into diffs, looks like you did not run the code locally, since updated helper uses non-existing fields from Library after the updates. Sorry that we do not have workflows setup on the CI for basic code checks which will help you to caught that, will add that soon!
| lastPublish, | ||
| maintained, | ||
| reactNativeVersion, |
There was a problem hiding this comment.
These does not exist, you can see what information is available by looking at Library type.
| if (maintained === false) { | ||
| return NewArchSupportStatus.Unsupported; | ||
| } |
There was a problem hiding this comment.
Unfortunately, this is an incorrect assumption. Unmaintained libraries can support New Architecture in multiple cases, like:
- deprecation has happen after New Architecture introduction
- package is JS only (does not include native code)
- package has been marked as unmaintend due to name change
| const newArchIntroDate = new Date('2022-04-01'); | ||
| if (lastPublish && new Date(lastPublish) < newArchIntroDate) { | ||
| return NewArchSupportStatus.Unsupported; | ||
| } |
There was a problem hiding this comment.
This is not true for package not including any native code (JS only).
| if (reactNativeVersion && isVersionOlderThan(reactNativeVersion, '0.68.0')) { | ||
| return NewArchSupportStatus.Unsupported; | ||
| } |
There was a problem hiding this comment.
We do not have the React Native version information at this time. We were experimenting with this idea, we can extract the information for package.json file, but the problem is with the data itself, which package developers provide, and we have concluded that we cannot rely globally on that information.
What we have found out - often React Native is not specified as regular, peer or dev dependency, often when specified semver is ster to * which does not give us any viable information, there are also cases when this information is set once, and never updated, so we have entries like >0.49.
|
With #1539 shipped, when you rebase your PR you should be able to see the missing data issues on the CI. |
|
Hello @Simek, Thanks for the feedback! Regarding the libraries I’ve found to be either not working or unstable with the new architecture, here are a few examples:
if you visit https://reactnative.directory/?order=updated&direction=ascending&search=expo&newArchitecture=true you’ll see that almost every result there is not working on the new architecture. This should provide a clearer picture of the widespread issues I’m trying to address. Don't you think that the logic if (expoGo === true) might no longer be considered valid, especially after the recent changes in Expo (versions 51 and 52) and React Native? Regarding the current solution, I realize that my initial approach could be improved. Thanks for pointing out the issue with the helper using non-existing fields after the updates. I’ll keep you updated with my progress! have nice day Simek |
|
Hey @nympheastudio, thanks for the answer, and a bit more context! 👍 I have made a quick test project with some of the libraries from the list you have provided, which does not require additional setup steps, and I can definitely tell that they are working with New Architecture enabled correctly. They are mostly non-native code libraries, so this is expected. Here is the source code:
Preview video from the simulator: rec.mp4This research effort makes me think that there might something wrong with the app you trying to use those packages in, not with the packages itself.
Nope, JavaScript only packages should work the same in older and the newest React Native versions. The New Architecture changes should not affect the compatibility of these packages. There might be a case or two overall where some other breaking changes (not related to New Architecture effort) have been made to React Native core, which resulted in libraries incompatibility, but this is a natural thing with versioning. In general, if you are experiencing any problem with packages in ecosystem, the best approach is to work on minimal reproducible example, and then share that or report as an issue in the given package repository. You can read a bit more about it here: Only if we have confidence, or an example showing that the given package does not work with New Architecture, we should alter the state in directory. When it comes to the detection mechanism, I think it's in a quite good spot when it comes to automarking rules, but I you will be able to provide the more compelling arguments, with examples showing false-positive reports that are recurring, I'm happy to alter the logic we use. |
|
Hey @Simek, thanks again for your response and for testing this out! After testing your sample project, I noticed that Cluster is not working. The issue seems to come from react-native-maps, which, as mentioned in this issue, does not yet support the New Architecture. Also, I’d like to apologize for my previous doubts about react-native-responsive-image and react-native-buttonex—after further testing, they do indeed seem to support the New Architecture properly. The main concern here is that a clustering library is marked as compatible with the New Architecture, even though it relies on react-native-maps, which is not yet fully compatible. While the package may load under Expo + new Arch and not crash, the fact that it cannot be used due to its dependency raises a question: wouldn’t it be misleading for readers of the directory to see it marked as compatible when, in practice, it doesn’t function properly? To highlight this issue further, I’m going to test more packages listed in [reactnative.directory](https://reactnative.directory/?order=updated&direction=ascending&search=expo&newArchitecture=true) to see if similar cases arise. I'll provide more detailed feedback soon. Thanks and have a good day. |
This pull request introduces improvements to the detection of libraries supporting the new architecture in React Native. Specifically, it addresses issues where some libraries were incorrectly marked as supporting the new architecture, even though they do not.
📝 Why & how
This update ensures that libraries which are outdated, unmaintained, or incompatible with React Native's new architecture are properly marked as unsupported. It also provides more accurate detection of new architecture support based on library metadata, React Native version, and maintenance status.
🔢 Example of bug :
On https://reactnative.directory/?search=maps, react-native-map-clustering is incorrectly tagged with "New Architecture," even though it doesn't actually support it.