feat: add RawPropsParser as optional parameter to Concrete-/ComponentDescriptor#48232
Closed
hannojg wants to merge 1 commit into
Closed
Conversation
facebook-github-bot
added
the
CLA Signed
facebook-github-bot
added
the
Shared with Meta
javache
approved these changes
Dec 12, 2024
Contributor
|
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
Collaborator
|
This pull request was successfully merged by @hannojg in 9dce262 When will my fix make it into a release? | How to file a pick request? |
facebook-github-bot
pushed a commit
that referenced
this pull request
Dec 12, 2024
Summary: In this PR we added a change that allows the RawPropsParser to construct its RawValues directly from the `jsi::Value` instead of converting it to `folly::dynamic` first. We added a global feature flag to turn this on, however, for migrations it might be better to use this functionality as an opt-in on a component basis. With this change `ComponentDescriptors` can now create their RawPropsParser instance with the `useRawPropsJsiValue` flag to opt into it. (Note: a few more changes are needed to make this accessible to the `ComponentDescriptor`, for which I opened [this follow up PR here](#48232)) ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [INTERNAL] [ADDED] - Added `useRawPropsJsiValue` parameter to `RawPropsParser` to opt into skipping folly::dynamic conversions during prop parsing. Pull Request resolved: #48231 Test Plan: Internal change / just make sure all tests are passing. Reviewed By: NickGerleman Differential Revision: D67139641 Pulled By: javache fbshipit-source-id: 5b243edb8149870aad0a5a1b3998ee67997783d7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
In this PR we introduced a new mechanism for
RawPropsParserto construct its RawValues directly fromjsi::Valueinstead of converting it tofolly::dynamicfirst:RawValuedirectly fromjsi::Value#48047In this PR we added a parameter to
RawPropsParserto opt-into using the above described mechanism:RawPropsParseradduseRawPropsJsiValueparameter #48231Whats missing is that
RawPropsParserwas default constructed inComponentProviderand there is no way to pass a custom instance (where you'd for example set the above described parameter). This PR adds support for that.Changelog:
[INTERNAL] [ADDED] - Add
RawPropsParseras optional parameter to Concrete-/ComponentDescriptorTest Plan:
Internal change, just make sure all CI tests are passing.