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

[titoBouzout]

Summary

Related to #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

solid/packages/solid/src/reactive/signal.ts

Line 1749 in 0af2c2b

Array.isArray(result) ? results.push.apply(results, result) : results.push(result);

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 ? 🤔

solid/packages/solid/bench/libraries/s-mod.cjs

Line 293 in 0af2c2b

Array.isArray(result) ? results.push.apply(results, result) : results.push(result);

How did you test this change?

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

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3d2168d

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