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]

🦋 Changeset detected

Latest commit: d598b4b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
solid-js	Patch
test-integration	Patch

Not sure what this means? Click here to learn what changesets are.

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

[coveralls]

Coverage Report for CI Build 25889426383

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.5%) to 85.888%

Details

• Coverage increased (+0.5%) from the base build.
• Patch coverage: 3 uncovered changes across 1 file (8 of 11 lines covered, 72.73%).
• 3 coverage regressions across 2 files.

Uncovered Changes

File	Changed	Covered	%
packages/solid/src/reactive/signal.ts	11	8	72.73%

Coverage Regressions

3 previously-covered lines in 2 files lost coverage.

File	Lines Losing Coverage	Coverage
packages/solid/store/src/store.ts	2	97.57%
packages/solid/src/reactive/scheduler.ts	1	80.39%

---

Coverage Stats

Relevant Lines:	2709
Covered Lines:	2385
Line Coverage:	88.04%
Relevant Branches:	1153
Covered Branches:	932
Branch Coverage:	80.83%
Branches in Coverage %:	Yes
Coverage Strength:	64.53 hits per line

---

💛 - Coveralls