fix "Maximum call stack size exceeded" on `resolveChildren`s `results.push.apply(results, result)`
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:
- if a component can return data, say an array with 200k items, then we want to fix it.
- for performance, I suppose (didn't measure), that checking for the
lengthis negligible, (as in no performance impact) - 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.
⚠️ 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