Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dispatch commands to both UIManagers from both renderers #17211

Merged
merged 2 commits into from
Oct 30, 2019

Conversation

elicwhite
Copy link
Member

ReactNative's internal module for codegenNativeCommands needs to dispatch to Fabric or paper depending on the node it was passed.

This makes the internal Renderer's exported dispatchCommand method both support components created with either renderer.

There was a test file ReactFabricAndNative-test.internal.js that tests components created with Fabric passed to the Paper renderer.

@sizebot
Copy link

sizebot commented Oct 29, 2019

Size changes (experimental)

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 85dcaff

@sizebot
Copy link

sizebot commented Oct 29, 2019

Size changes (stable)

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 85dcaff

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to cause any early require problems such as both UI managers eagerly instantiating or is that all fixed?

@elicwhite
Copy link
Member Author

Since it is accessing the fabricUIManager directly from the global we shouldn't have any of those problems. When I forgot to do that and instead defined it at the top of the file, the tests failed. So I think this should be safe.

@elicwhite elicwhite merged commit 8eee0eb into facebook:master Oct 30, 2019
@elicwhite elicwhite deleted the dispatch-fabric branch October 30, 2019 01:16
elicwhite added a commit to elicwhite/react that referenced this pull request Oct 30, 2019
)

* Dispatch commands to both UIManagers from both renderers

* Merge test files
elicwhite added a commit to elicwhite/react that referenced this pull request Oct 31, 2019
elicwhite added a commit that referenced this pull request Oct 31, 2019
elicwhite added a commit to elicwhite/react that referenced this pull request Oct 31, 2019
)

* Dispatch commands to both UIManagers from both renderers

* Merge test files
elicwhite added a commit to elicwhite/react that referenced this pull request Oct 31, 2019
trueadm pushed a commit to trueadm/react that referenced this pull request Nov 4, 2019
)

* Dispatch commands to both UIManagers from both renderers

* Merge test files
trueadm pushed a commit to trueadm/react that referenced this pull request Nov 4, 2019
elicwhite added a commit to elicwhite/react that referenced this pull request Jan 7, 2020
elicwhite added a commit that referenced this pull request Jan 8, 2020
…ers (#17211)" (#17232)" (#17799)

* Revert "Revert "Dispatch commands to both UIManagers from both renderers (#17211)" (#17232)"

This reverts commit d0fc0ba.

* Clean up another __DEV__ warning check
elicwhite added a commit to elicwhite/react that referenced this pull request Jan 8, 2020
…ers (facebook#17211)" (facebook#17232)" (facebook#17799)

* Revert "Revert "Dispatch commands to both UIManagers from both renderers (facebook#17211)" (facebook#17232)"

This reverts commit d0fc0ba.

* Clean up another __DEV__ warning check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants