fix(layout): enforce height on items in horizontal lists

[shopify-github-actions-access]

Description

In horizontal FlashList, items had no enforced height dimension (enforcedHeight was never set in LinearLayoutManager). This caused nested ScrollViews to receive invalid layout metrics with infinity/NaN values, making them unable to render properly.

The fix adds enforcedHeight = true for horizontal layouts (symmetric with how vertical layouts use enforcedWidth) and sets item height to the bounded container size when known. This gives nested ScrollViews a definite height constraint.

Fixes #1924

Reviewers' hat-rack 🎩

• The change in estimateLayout mirrors the existing enforcedWidth = !this.horizontal pattern
• The processLayoutInfo change ensures measured heights respect the bounded size for horizontal lists (similar to how width uses boundedSize for vertical lists)
• Consider edge cases: horizontal lists without explicit parent height (uses normalizeLayoutHeights path, unaffected by the hasSize guard)

Test plan

• Unit tests pass (yarn test)
• Type check passes (yarn type-check)
• Lint passes (yarn lint)
• Verified on iOS simulator
• No regressions on related screens

[Copilot]

Pull request overview

This PR fixes incorrect cross-axis sizing for horizontal FlashList items by enforcing a bounded height, preventing nested ScrollView children from receiving invalid layout constraints (Infinity/NaN) and failing to render.

Changes:

• Forces measured item height to the horizontal bounded container size when available (boundedSize).
• Introduces enforcedHeight = true for horizontal layouts (symmetric with enforcedWidth in vertical layouts).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

estimateLayout now sets enforcedHeight = this.horizontal, which makes layout.height a hard constraint in horizontal mode. However, layout.height is still initialized to getEstimatedHeight(index) even when the cross-axis container size is already known (this.hasSize). This can end up enforcing the default estimate (e.g. 200) instead of the bounded container height, which undermines the intent of providing a definite height constraint early (before real measurements). Consider setting layout.height to this.boundedSize when this.horizontal && this.hasSize, mirroring how vertical mode uses boundedSize for width.

Suggested change

	layout.height = this.getEstimatedHeight(index);
	layout.height = this.horizontal && this.hasSize
	? this.boundedSize
	: this.getEstimatedHeight(index);

[Copilot]

This logic relies on this.hasSize to decide whether to force horizontal item height to boundedSize, but hasSize is only set in the constructor and is not updated when boundedSize changes in updateLayoutParams. If windowSize.height transitions from 0 to a real value after mount (common during initial layout), hasSize can remain false and this branch will never apply, leaving horizontal items without the intended enforced height. Consider updating this.hasSize = this.boundedSize > 0 inside updateLayoutParams whenever boundedSize is recalculated.

[Copilot]

The new horizontal-only behavior (enforcedHeight and height overriding to boundedSize) isn't covered by the current LinearLayoutManager unit tests. Adding assertions for layouts[i].enforcedHeight === true and that layouts[i].height equals the horizontal bounded container height (when windowSize.height is known) would help prevent regressions for the nested-ScrollView scenario this PR fixes.

[naqvitalha]

@agent review the feedback left by copilot

[github-actions]

Claude finished @naqvitalha's task in 1m 23s —— View job

---

Review of Copilot's feedback on PR #2205

• Read the changed file and surrounding context
• Evaluate each Copilot comment
• Post review summary

---

I reviewed all three Copilot comments against the actual code. Here's my assessment:

1. hasSize not updated in updateLayoutParams — Valid, this is a bug ⚠️

This is the most important finding. hasSize is set once in the constructor (line 29) and never updated. In updateLayoutParams (lines 36–53), boundedSize is recalculated but hasSize is not:

// updateLayoutParams updates boundedSize...
this.boundedSize = this.horizontal
  ? params.windowSize.height
  : params.windowSize.width;
// ...but never updates hasSize!

If the initial windowSize.height is 0 (common during initial RN layout), hasSize stays false permanently. This means the guard on line 70 (this.horizontal && this.hasSize) will never be true, and horizontal items will never get their height enforced to boundedSize — defeating the purpose of this PR.

Fix: Add this.hasSize = this.boundedSize > 0; inside updateLayoutParams, after boundedSize is recalculated (around line 42).

Fix this →

2. estimateLayout should use boundedSize for height when known — Valid, minor

In estimateLayout, layout.height always uses getEstimatedHeight(index) (line 87), even when the actual container height is known. Since enforcedHeight = true is now set on line 90, this means items are initially enforced to the estimated height rather than the actual container height.

This is self-correcting — processLayoutInfo (line 69–70) fixes it when real measurements arrive — but setting layout.height = this.boundedSize upfront when this.horizontal && this.hasSize would avoid an unnecessary layout pass. Copilot's suggestion is sound:

layout.height = this.horizontal && this.hasSize
  ? this.boundedSize
  : this.getEstimatedHeight(index);

Worth doing but lower priority than the hasSize bug above.

3. Missing unit test coverage — Valid, good practice

The existing horizontal tests don't assert on enforcedHeight or verify that layout.height === boundedSize after processLayoutInfo. Adding a test case for the "horizontal list with known height enforces boundedSize on items" scenario would lock in the intended behavior. Given this PR's goal is specifically about enforcing height, a test is warranted.

---

Summary

#	Copilot Comment	Verdict	Severity
1	hasSize not updated in updateLayoutParams	Valid bug — the fix won't work when initial height is 0	High
2	estimateLayout should use boundedSize	Valid improvement — avoids extra layout pass	Low
3	Missing test coverage	Valid — should add assertions for new behavior	Medium

Recommendation: Comment 1 should be addressed before merging — it's a correctness issue that can make the fix ineffective in a common scenario (initial layout with 0 height). Comments 2 and 3 are nice-to-haves that would strengthen the PR.

---

[naqvitalha]

@agent we do assume that once hasSize is set, it's always true.

[github-actions]

Claude finished @naqvitalha's task in 41s —— View job

---

• Understand the clarification from maintainer
• Reassess review findings in light of the hasSize assumption

---

Updated Assessment

Thanks for the clarification. With the assumption that hasSize is only ever false → true (never reverts), the concern about updateLayoutParams not updating it is valid only for the case where the initial windowSize.height is 0 and later becomes non-zero. Let me check whether that scenario is actually handled.

Looking at the code:

• Constructor (line 29): hasSize = boundedSize > 0
• updateLayoutParams (lines 36–53): recalculates boundedSize and calls recomputeLayouts if it changed, but does not update hasSize

So if a horizontal list mounts with windowSize.height = 0 (which React Native can do during initial layout), hasSize stays false forever. The guard on line 70 (this.horizontal && this.hasSize) and line 73 (this.horizontal && !this.hasSize) would permanently take the !hasSize path, using normalizeLayoutHeights instead of enforcing boundedSize.

Given your assumption that once set it's always true: the fix is simply to also set hasSize when boundedSize becomes positive in updateLayoutParams. This is consistent with the "once true, always true" invariant — it just ensures the flag gets set even when the initial size was 0.

A one-line addition after line 42 would do it:

this.hasSize = this.hasSize || this.boundedSize > 0;

This preserves the "never goes back to false" invariant while handling the late-arriving size. Without it, the PR's fix is ineffective when windowSize.height starts at 0.

The other two Copilot suggestions (using boundedSize in estimateLayout, adding tests) are lower priority but still good improvements.

---