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

Yoga: call getMeasure even when YGMeasureModeExactly #100

Merged
merged 3 commits into from
Jul 2, 2019
Merged

Yoga: call getMeasure even when YGMeasureModeExactly #100

merged 3 commits into from
Jul 2, 2019

Conversation

sagokhal
Copy link

@sagokhal sagokhal commented Jul 2, 2019

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Description of changes

Yoga leaf nodes in Win32 can be sub-trees which rely on the measure call (getMeasure) to perform the sub-tree layout passes.

Focus areas to test

Yoga layout

Microsoft Reviewers: Open in CodeFlow

@sagokhal sagokhal requested a review from acoates-ms as a code owner July 2, 2019 00:05
@ghost
Copy link

ghost commented Jul 2, 2019

Hello @acoates-ms!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 60 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@pull-bot
Copy link

pull-bot commented Jul 2, 2019

Messages
📖 📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.
📖 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.
📖

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

Generated by 🚫 dangerJS against d0db29b

@acoates-ms
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@acoates-ms acoates-ms merged commit 3432672 into microsoft:master Jul 2, 2019
NickGerleman pushed a commit to NickGerleman/react-native that referenced this pull request Oct 29, 2024
…acebook#46702)

Summary:
Pull Request resolved: facebook#46702

In facebook#44188, we've started combining multiple transactions in a single transaction, to meet React's atomicity requirements, while also dealing with the constraints of Android's Fabric implementation.

This revealed a bug where in some scenarios (especially when using transitions), a node may be deleted and created during the same transaction. The current implementation of FabricMountingManager assumes it can safely reorder some operations, which it does to optimize the size of IntBufferBatch mount items. This is however incorrect and unsafe when multiple transactions are merged.

**Example:**

Differentiator output:

```
# Transaction 1
Remove microsoft#100 from microsoft#11
Delete microsoft#100

# Transaction 2
Create microsoft#100
Insert microsoft#100 into microsoft#11
```
FabricMountingManager output
```
Remove microsoft#100 from microsoft#11
Insert microsoft#100 into microsoft#11
Delete microsoft#100
```

Note that the create action is also skipped, because we only update `allocatedViewTags` after processing all mutations, leading FabricMountingManager to assume creation is not required.

This leads to an invalid state in SurfaceMountingManager, which will be surfaced as a crash in `getViewState` on the next mutation that interacts with these views.

Changelog: [Android][Fixed] Fix crash in getViewState when using suspense fallbacks.

Reviewed By: sammy-SC

Differential Revision: D63148523

fbshipit-source-id: 07ae26b2f7b7eba1b9784041dd3059b0956c035e
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.

3 participants