Skip to content

fix(Table): guard rowExpansion tree walks against cyclic data#4070

Open
AKnassa wants to merge 2 commits into
facebook:mainfrom
AKnassa:ak-3971-rowexpansion-cycle
Open

fix(Table): guard rowExpansion tree walks against cyclic data#4070
AKnassa wants to merge 2 commits into
facebook:mainfrom
AKnassa:ak-3971-rowexpansion-cycle

Conversation

@AKnassa

@AKnassa AKnassa commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What this does

Stops the Table's row-expansion feature from crashing when the row data accidentally contains a loop (a row that ends up being its own ancestor).

Why

With cyclic data, the three internal tree walks recursed forever and crashed with a stack overflow — one of them even with nothing expanded. This is the cycle-guard follow-up humbertovirtudes prescribed in #3971, and the fix pattern mirrors the guarded tree walks already reviewed in #3789.

What changed

  • Each of the three walks (depth map, flattened rows, expand-all keys) now tracks the keys on its current path and simply skips an edge that loops back.
  • 3 new tests reproduce the crash first (self-referencing row, deeper cycle, expand-all with nothing expanded); a 4th pins the guard's ancestor-path semantics on shared-subtree (DAG) data — a shared child must flatten under each expanded parent, which a global visited-set guard would get wrong.
  • Behavior for normal, non-cyclic data is unchanged.
  • The four formatting-only hunks in the plugin section are the repo's own prettier normalizing pre-existing formatting on main (lint-staged applies it on commit) — safe to skip in review.

How to see it

pnpm exec vitest run packages/core/src/Table/plugins/rowExpansion/useTableRowExpansion.test.tsx — 13/13 pass (the 3 cyclic ones crash with RangeError before the fix; the DAG test fails if the guard is degraded to a visited-set). Full core suite green.

Fixes #3971

Cyclic or self-referential row data crashed useTableRowExpansionState
with a stack overflow: all three tree walks (depthMap, flattened data,
allExpandableKeys) recursed without ancestor tracking. Track the
ancestor-key path in a Set and skip the cyclic edge, keeping
first-encountered depth semantics. The allExpandableKeys walk crashed
even with nothing expanded, since it recurses unconditionally.

Fixes facebook#3971

Claude-Session: https://claude.ai/code/session_01P9qSoGWZPPp9pUNNRUSwkM
@vercel

vercel Bot commented Jul 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 19, 2026 2:06am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 19, 2026
@github-actions github-actions Bot added community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge labels Jul 19, 2026
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.7KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

…ard (facebook#3971)

The cycle tests couldn't tell the prescribed ancestor-path guard from a
global visited-set (which would silently drop a shared child under its
second expanded parent on acyclic DAG data). Replace the redundant
first-depth test with a DAG pin that fails on the visited-set variant,
and retitle the once-each claims to what the assertions actually prove
(termination).
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 Meta Open Source bot. community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

useTableRowExpansion: add cycle guard to tree walks (stack overflow on cyclic data)

1 participant