Skip to content

fix: treat opaque arrays like opaque objects - #669

Open
BitPhinix wants to merge 1 commit into
LegendApp:mainfrom
BitPhinix:opaque-arrays
Open

BitPhinix wants to merge 1 commit into
LegendApp:mainfrom
BitPhinix:opaque-arrays

Conversation

@BitPhinix

Copy link
Copy Markdown

ObservableHint.opaque has no effect on arrays. updateNodes and the proxy get trap only check the hint behind isObject(), which excludes arrays. An opaque-hinted array is still walked and gets child nodes for every element.

The types already treat an opaque array as a primitive. In observable({ arr: ObservableHint.opaque([...]) }), arr[0] and arr.push don't type-check, so the runtime was the only part that disagreed.

Change

Check the opaque hint for arrays too, in the same (isObject(x) || isArray(x)) form that extractFunctionsAndComputeds already uses. Setting an opaque array now notifies listeners once and creates no child nodes. Reading an index through the proxy returns the raw element, like a property of an opaque object.

Why it matters

We hit this in a React Native app. It stored a list of Relay fragment refs with ObservableHint.opaque(messages). Each ref carries the Relay query AST, so every element grew about 1,500 child nodes. With ~1,000 loaded messages that was ~400 MB of JS heap, and each set re-diffed the whole tree, taking up to ~250 ms. That was enough to crash the app on long lists.

Testing

  • Added opaque array should be handled like primitives. It fails on main (the node has children) and passes with this change.
  • tsc --noEmit, ESLint and Prettier pass on the changed lines.
  • bun test --timeout 50 has the same 19 failures as unmodified main, mostly sync-retry tests that run past the 50 ms timeout, so nothing new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant