Fixed ReactNativeFiber event handling regression #8959
Conversation
Also added test coverage to ReactNativeEvents-test for Fiber
The test now mocks |
|
Got it, but that's false in the repo right? Let's mock RN always: but it seems like we should also make that depend on the feature flag by adding a ReactNativeFeatureFlags mock in the same file. Can you do that? |
Updated the test-framework-setup file so that record-tests runs native tests against ReactNativeFiber. ReactComponentTreeHook native tests all now fail but that's expected.
| @@ -45,17 +47,17 @@ describe('ReactNative', () => { | |||
|
|
|||
| ReactNative.render(<View foo="foo" />, 11); | |||
|
|
|||
| expect(UIManager.createView.mock.calls.length).toBe(2); | |||
| expect(UIManager.setChildren.mock.calls.length).toBe(2); | |||
| expect(UIManager.createView.mock.calls.length).toBe(1); | |||
There was a problem hiding this comment.
I see changes to this test but no changes to how stack works. So how did this pass before vs. now? EDIT: Oh nvm. I get it. This used to be dependent on module state between resets.
There was a problem hiding this comment.
Yup. Noticed this when fit() running the test.
| if (parentInstance._children.length === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| // Map from child objects to native tags. | ||
| // Either way we need to pass a copy of the Array to prevent it from being frozen. | ||
| const nativeTags = parentInstance._children.map( |
There was a problem hiding this comment.
Unrelated: There's some slow paths here that we should look into optimizing.
Fixed event handling regression introduced in b354db2 and added test coverage to ReactNativeEvents-test for fiber.