[table] Fix infinite loop in eachUniqueFullRow/Column for unbounded intervals#8134
Conversation
…terval Regions.eachUniqueFullRow and Regions.eachUniqueFullColumn iterate from the interval start to its end with a counting loop. When a full-row or full-column region has an unbounded end (e.g. rows of [1, Infinity], which can result from shift-clicking a row or column header and is then passed to the public API in an onSelection handler), the loop never terminates and the tab hangs or throws once the accumulator array overflows. Skip regions whose interval end is not finite so the iteration cannot run unbounded. Bounded regions in the same array are still enumerated normally. Fixes palantir#6383.
|
Thanks for your interest in palantir/blueprint, @adityasingh2400! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
Generate changelog in
|
✅ Successfully generated changelog entry!Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. 📋Changelog Preview🐛 Fixes
|
| fix: | ||
| description: "`Regions.eachUniqueFullRow` and `Regions.eachUniqueFullColumn` no longer hang on regions with an unbounded interval" | ||
| links: | ||
| - https://github.com/palantir/blueprint/pull/8134 |
There was a problem hiding this comment.
Could you remove this changelog entry? Per our contribution conventions, changelog entries aren't authored as part of the PR, they're generated separately. Thanks!
There was a problem hiding this comment.
Done, removed it. Thanks for the pointer, I did not realize changelog entries are generated separately rather than authored in the PR. Apologies for the extra file.
ggdouglas
left a comment
ggdouglas
left a comment
There was a problem hiding this comment.
Verified locally: repros the RangeError on develop and returns instantly on this branch. LGTM, thanks!
|
Thanks for verifying it locally and for the review, much appreciated. |
3 participants
Fixes #6383
Checklist
Changes proposed in this pull request:
Regions.eachUniqueFullRowandRegions.eachUniqueFullColumniterate from the interval start to its end with a countingforloop. When a full-row or full-column region has an unbounded end (for examplerows: [1, Infinity]), the loop never terminates. As noted in #6383, such a region can be produced by shift-clicking a row header and then passed to the public API inside anonSelectionhandler, at which point the page hangs (or eventually throwsRangeError: Invalid array lengthonce an accumulating array overflows).This change skips any full-row or full-column region whose interval end is not finite, so the iteration cannot run unbounded. Bounded regions in the same array continue to be enumerated as before. The maintainer's comment on the issue pointed at these exact loops and suggested the function should short circuit, which is what this does.
Reviewers should focus on:
Whether silently skipping an unbounded region is the preferred behavior versus clamping it to the table bounds. These iteration helpers do not receive
numRows/numCols, so skipping is the least surprising option without changing the signatures. The internal callers intable.tsx(handleRowHeightChanged/handleColumnWidthChanged) write into bounded arrays, so they are unaffected in practice; the user-facing impact is in the publicRegionsAPI.Verification:
nx run @blueprintjs/table:compile,vitest run regions.test.ts(38 passing), andeslinton the changed files all pass. The added test "does not hang on unbounded intervals" fails without the fix and passes with it.