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

[ReactNative] Maintain order of bridge calls #2488

Closed
wants to merge 1 commit into from
Closed

[ReactNative] Maintain order of bridge calls #2488

wants to merge 1 commit into from

Conversation

RoboTeddy
Copy link
Contributor

When bridge calls are made, they should be dispatched to their
destination GCD queue in the same order they were made. (It
looks like this invariant broke in 336e18d, which caused call
order to depend on the iteration of NSMapTable keys
whenever there are calls to multiple modules that share a queue)

Fixes #1941 (in which RCTUIManager createView addUIBlock
blocks were sometimes running after other blocks that depended
on them)

I'm a react-native/iOS/objc newbie, so please excuse any
ignorance this commit may well contain :)

When bridge calls are made, they should be dispatched to their
destination GCD queue in the same order they were made.
(It looks like this invariant broke in 336e18d, which caused
call order to depend on the iteration of NSMapTable keys)

Fixes #1941 (in that bug, createView addUIBlock blocks were
sometimes running after other blocks that depend on them)

I'm a react-native/iOS/objc newbie, so please excuse any
ignorance this commit may well contain :)
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 29, 2015
@RoboTeddy
Copy link
Contributor Author

(looks like travis failed on something unrelated... closing and opening PR to see if it gives him a kick)

@RoboTeddy RoboTeddy closed this Aug 30, 2015
@RoboTeddy RoboTeddy reopened this Aug 30, 2015
@RoboTeddy
Copy link
Contributor Author

cc @tadeuzagallo (looks like you've written a lot of bridge code!)

@tadeuzagallo
Copy link
Contributor

That makes sense. Since every module has its own queue by default, it shouldn't matter in most cases, but a clear example is that all view managers share the same queue.
Thanks for catching it and pinging me, feel free to ping me on any other bridge/perf related issues, sometimes I really don't see it.

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/675599992541433/int_phab to review.

@sahrens sahrens closed this in 2b3a4bd Sep 4, 2015
MattFoley pushed a commit to skillz/react-native that referenced this pull request Nov 10, 2015
Summary:
When bridge calls are made, they should be dispatched to their
destination GCD queue in the same order they were made. (It
looks like this invariant broke in 336e18d, which caused call
order to depend on the iteration of `NSMapTable` keys
whenever there are calls to multiple modules that share a queue)

Fixes facebook#1941 (in which RCTUIManager createView addUIBlock
blocks were sometimes running after other blocks that depended
on them)

I'm a react-native/iOS/objc newbie, so please excuse any
ignorance this commit may well contain :)
Closes facebook#2488
Github Author: Ted Suzman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants