solid icon indicating copy to clipboard operation
solid copied to clipboard

fix "Maximum call stack size exceeded" on `resolveChildren`s `results.push.apply(results, result)`

Open titoBouzout opened this issue 4 months ago • 1 comments

Summary

Related to https://github.com/solidjs/solid/issues/2542

What happens is that the children helper crashes when trying to results.push.apply(results, [149k items]), as it overflows the max amount of arguments a function can have.

Line https://github.com/solidjs/solid/blob/0af2c2b4dc5f7da8022c16563c25d0687f68aff3/packages/solid/src/reactive/signal.ts#L1749

Now, if we want to fix this, I'm honestly not sure, considerations:

  1. if a component can return data, say an array with 200k items, then we want to fix it.
  2. for performance, I suppose (didn't measure), that checking for the length is negligible, (as in no performance impact)
  3. this fix probably helps to get something dirty done fast, to then be edited to do it the right way, say use something like TanStack Table or TanStack Virtual

Think point 1 gets me. If a component that can return data ends somehow being resolved by the children helper, then I suppose we should fix this.

  • Repro of the issue https://playground.solidjs.com/anonymous/8571b506-158c-43ab-84a1-2c21de773f3e
  • Repro with just the children helper https://playground.solidjs.com/anonymous/631ec862-0d2a-4b2a-a70e-a9844d1fb821
  • Repro without solid https://playground.solidjs.com/anonymous/4894991e-31ac-48db-a00b-d271865f1c84

Question: should also apply the edit to ? 🤔 https://github.com/solidjs/solid/blob/0af2c2b4dc5f7da8022c16563c25d0687f68aff3/packages/solid/bench/libraries/s-mod.cjs#L293

How did you test this change?

Test passes, I also tested it by hacking the solution in vites output.

titoBouzout avatar Sep 07 '25 14:09 titoBouzout

⚠️ No Changeset found

Latest commit: 3d2168d7f1670b43881a297d95b7985980743690

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Sep 07 '25 14:09 changeset-bot[bot]