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

basic New Arch detection and new status #870

Merged
merged 4 commits into from
Aug 31, 2022
Merged

Conversation

Simek
Copy link
Member

@Simek Simek commented Aug 21, 2022

📝 Why & how

Refs #830

This PR add a basic New Architecture support detection based on the present of the Codegen related field in the library package.json.

The status in display in the secondary metadata row and links to the New Architecture intro docs on the React Native docs website, which might help the directory users to understand what this term mean, if they sees it in library box.


Things to consider before converting from draft:

  • should we allow users to manually add this flag to the JSON entries?
  • should we add filter for this status (not sure if it is a good idea, until we are sure that this basic method won't cover almost all of the cases)

📸 Preview

https://react-native-directory-okpe29xli-rndir.vercel.app/

Library Box

Screenshot 2022-08-21 191622

✅ Checklist

  • Added library to react-native-libraries.json
  • Updated library in react-native-libraries.json
  • Documented in this PR how to use the feature or replicate the bug.
  • Documented in this PR how you fixed or created the feature.

@brentvatne
Copy link
Contributor

should we allow users to manually add this flag to the JSON entries?

yeah, we should! we take a different approach to supporting fabric in expo modules and the approach in this pr won't detect it

@brentvatne
Copy link
Contributor

should we add filter for this status (not sure if it is a good idea, until we are sure that this basic method won't cover almost all of the cases)

this could be useful. if we identify that something is wrong, we can adjust as we go

Copy link
Collaborator

@jonsamp jonsamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this meta data!

@Simek Simek marked this pull request as ready for review August 28, 2022 19:07
@Simek Simek linked an issue Aug 28, 2022 that may be closed by this pull request
@cipolleschi
Copy link

should we allow users to manually add this flag to the JSON entries?

What do you mean here? Lib maintainer has to manually add this entry in the package.json, otherwise the app can't generate the code it needs.

should we add filter for this status

Which statuses are you considering? Something like: In development, iOS done, Android not done, or other.
I guess that for the sake of the user and of simplicity, we should only use a simple boolean: whether the latest release support the new architecture or not.

@Simek Simek added deploy and removed deploy labels Aug 29, 2022
@tomekzaw
Copy link
Contributor

should we allow users to manually add this flag to the JSON entries?

React Native Reanimated also supports the New Architecture but doesn't have any native components or TurboModules, so the possibility to override this flag in react-native-libraries.json would be very appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supports New Architecture flag
5 participants