react-native-heap
react-native-heap copied to clipboard
Add new HOC for instrumenting components manually
Description
What does this PR add/fix/change?
Our project uses Pickers quite extensively, from https://github.com/react-native-picker/picker.
However, using these pickers has one limitation - unlike Touchables, etc. they are not instrumented with Heap for autocapture.
This means that we need to try and do it ourselves, either with custom events or another method. This is less than ideal, and is prone to errors.
This PR instead adds a new HOC that can be used to wrap the picker and wire it up with autocapture, like so:
import Heap from '@heap/react-native-heap';
import { Picker } from '@react-native-picker/picker';
const InstrumentedPicker = Heap.withHeapAutocapture(Picker, 'onFocus');
Previously, I had proposed a hard-coded solution (https://github.com/heap/react-native-heap/pull/265), but I found that almost immediately lacking. This instead covers far more cases.
Once it has been instrumented, whenever the picker opens (onFocus) we get a touch event auto captured in Heap. This then allows us to define our events directly in Heap with ease!
While working on this, we also had need to instrument some native components that, while not Touchables exactly, have onPress as a prop and behave in a similar way. To get this, we took a similar approach of using the HOC. This PR adds those HOCs to index.d.ts to ensure they can be used.
Is there anything reviewers should pay special attention to?
I cloned a lot of the work in withHeapFocusableAutocapture from prior work - please let me know if it is correct.
I also tried my best to define the HOCs in TypeScript, but I may have missed something there too.
Are there any breaking changes?
No, these are only additions.
Test Plan
How were these changes tested?
I applied these changes to my local project with patch-package and verified that:
- the instrumentation on the picker worked correctly
- all types passed type checking
Were additional test cases added?
No, I did not see any test cases for the other HOCs, so I have not here. Please let me know what to cover and I will give it a try.
Was there manual testing?
Yes, I tested manually in my project.
Checklist
- [ ] Detox tests pass (only Heap employees are able run these)
- [x] If this is a bugfix/feature, the changelog has been updated
Hi, just commenting to let you know that I see this PR and it looks promising. There's currently some internal blockage on accepting PRs, but when that clears I'll look at this.
Hi, just commenting to let you know that I see this PR and it looks promising. There's currently some internal blockage on accepting PRs, but when that clears I'll look at this.
Hi @bnickel I know it has been a while, but is the internal blockage resolved?