Changes to existing files to prep for hermes support #3790
Merged
Conversation
This file contains 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
Main thing is that some types are not default constructible and have no null state. Additionally, some types are no longer copyable.
RedBeard0531
commented
Jun 7, 2021
| @@ -270,8 +276,14 @@ struct Object { | |||
| static ValueType get_property(ContextType c, const ObjectType &o, const std::string &s) { return get_property(c, o, StringData(s)); } | |||
| static ValueType get_property(ContextType, const ObjectType &, const String<T> &); | |||
| static ValueType get_property(ContextType, const ObjectType &, uint32_t); | |||
| static void set_property(ContextType, const ObjectType &, const String<T> &, const ValueType &, PropertyAttributes attributes = None); | |||
| static void set_property(ContextType, const ObjectType &, uint32_t, const ValueType &); | |||
| static void set_property(ContextType ctx, ObjectType&& obj, const String<T>& field, const ValueType& val, PropertyAttributes attributes = None) { | |||
There was a problem hiding this comment.
Explanation, since it isn't obvious what changed here or why: JSI doesn't support setting properties on const Objects, so I had to remove const here. But you can't pass a temporary to a non-const ref argument, so just doing that prevents things like set_property(ctx, get_global(), ...), which we do in a few places. So I added Object&& overloads, that just directly call the Object& overload.
kneth
approved these changes
Jun 8, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Main thing is that some types are not default constructible and have no null
state. Additionally, some types are no longer copyable.
☑️ ToDos
(Removing all TODOs since this is just an internal code change with no user impact on its own)
* [ ] 📝 Changelog entry * [ ] 📝 `Compatibility` label is updated or copied from previous entry * [ ] 🚦 Tests * [ ] 📝 Public documentation PR created or is not necessary * [ ] 💥 `Breaking` label has been applied or is not necessary- typescript definitions file is updated
- jsdoc files updated
- Chrome debug API is updated if API is available on React Native
If this PR adds or changes public API's: