Fix "Not implemented" exception on JSC #6028
Conversation
|
@tomekzaw It's for performance. Why would we surround it with the try-catch on every call, since we will know everything after the first call. |
|
@tjzel I wouldn't worry much about performance in this case, C++ SInce we know that some versions react-native-macos don't support Also, the names Finally, I don't like the fact that some unrelated files have been formatted, can we move them to a separate PR? |
|
Sorry about that, I added |
|
@mrousavy No worries |
|
@tomekzaw I read an interesting article about try/catch blocks performance from which it seems that try blocks are fast but the catch ones are expensive. Since these calls would constantly throw when Regarding I'll revert formatting and make the names more approachable. |
| void ShareableObject::runWithNativeStateProbe( | ||
| std::function<void()> &&block) { | ||
| try { | ||
| block(); |
There was a problem hiding this comment.
Why we need to pass an lambda here instead of just execute obj.setNativeState(rt, nativeState_);?
| try { | ||
| block(); | ||
| isNativeStateImplemented_ = IsNativeStateImplemented::Yes; | ||
| } catch (...) { |
There was a problem hiding this comment.
Does this 'Not implemented' exception have a specific type that we can implicitly catch here?
Summary
In some implementations of JSC, at least
react-native-macoshere native state methods are not implemented and they are throwing errors. This PR adds logic that will go around this issue, by probing thehasNativeStatemethod to determine whether it should use that.Fixes #5974
Test plan