Skip to content

Conversation

zhongwuzw
Copy link
Contributor

@zhongwuzw zhongwuzw commented Dec 25, 2018

Seems we lost handler of isBatchActive from RCTBatchedBridge a86171a to RCTCxxBridge 5bc7e39.

Changelog:

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge

Test Plan:

isBatchActive always equal to NO before this PR, after it, if batched message is being invoked, isBatchActive equal to YES.

@zhongwuzw zhongwuzw requested a review from mhorowitz as a code owner December 25, 2018 10:27
@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 Dec 25, 2018
@react-native-bot react-native-bot added the Platform: iOS iOS applications. label Dec 25, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cpojer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@cpojer
Copy link
Contributor

cpojer commented Jan 19, 2019

Hello @zhongwuzw,

it seems like this was removed intentionally because it was both unused and can lead to wrong information due to races. Can you explain your use case and reasoning for why you brought it back and why we should include this in React Native again? Thank you.

@zhongwuzw
Copy link
Contributor Author

@cpojer Hi, isBatchActive is used to check wether batch calls are running in setNeedsLayout of RCTUIManager currently, is this really has any race issue? 🤔 the setter do on JavaScript Thread, seems we don't need to worry about race condition?

Include this check can decrease layout process times because if there has batch calls, it would do layout end of it.

@cpojer
Copy link
Contributor

cpojer commented Jan 21, 2019

Got it, I'm gonna land it since there are still references to it in the code. If somebody deems it not useful, I'll let them remove it in the future across the whole codebase.

@react-native-bot
Copy link
Collaborator

@zhongwuzw merged commit d55558e into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Jan 21, 2019
@react-native-bot react-native-bot added the Merged This PR has been merged. label Jan 21, 2019
@zhongwuzw zhongwuzw deleted the fix_isBatchActive branch January 21, 2019 08:36
matt-oakes pushed a commit to matt-oakes/react-native that referenced this pull request Feb 7, 2019
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
aleclarson pushed a commit to aleclarson/react-native-macos that referenced this pull request Feb 23, 2019
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook/react-native@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook/react-native#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
aleclarson pushed a commit to aleclarson/react-native-macos that referenced this pull request Feb 27, 2019
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook/react-native@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook/react-native#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
aleclarson pushed a commit to aleclarson/react-native-macos that referenced this pull request Feb 27, 2019
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook/react-native@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook/react-native#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
aleclarson pushed a commit to ptmt/react-native-macos that referenced this pull request Feb 27, 2019
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook/react-native@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook/react-native#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
aleclarson pushed a commit to ptmt/react-native-macos that referenced this pull request Feb 27, 2019
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook/react-native@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook/react-native#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
enigmaticsoul216 added a commit to enigmaticsoul216/RN_macos that referenced this pull request Feb 2, 2025
Summary:
Seems we lost handler of `isBatchActive` from [RCTBatchedBridge a86171a](https://github.com/facebook/react-native/blob/a86171a48284c440226a79a26c6d6a4704d401e9/React/Base/RCTBatchedBridge.m) to [RCTCxxBridge 5bc7e39](facebook/react-native@b774820#diff-a2a67635fffd7b690d14dc17ae563a71).

Changelog:
----------

[iOS] [fixed] - Fix isBatchActive of RCTCxxBridge
Pull Request resolved: facebook/react-native#22785

Reviewed By: mhorowitz

Differential Revision: D13731897

Pulled By: cpojer

fbshipit-source-id: 8d6b85bcea8fe8997a93b4e1ac8b8007422ca20e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants