New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix] introduce helper method to prevent push maximum call stack size exceeded error #71
Conversation
… exceeded error Fixes Rich-Harris#40 Alternative to Rich-Harris#64 Part of sveltejs/svelte#4694
… exceeded error Part of sveltejs#4694 Also related: Rich-Harris/code-red#71
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. thanks for tackling this!
src/utils/push_array.js
Outdated
for (let i = 0; i < items.length; i++) { | ||
array.push(items[i]); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like there's whitespace before the }
which shouldn't be necessary. same above before the export
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm no longer able to format code since using Prettier 😄 Fixed
src/utils/push_array.js
Outdated
* Does `array.push` for all `items`. | ||
* Needed because `array.push(...items)` throws | ||
* "Maximum call stack size exceeded" when items | ||
* is too big of an array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems like a pretty short line wrap length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjusted 👍
As discussed in the maintainers meeting, this just introduces a simple
push_array
methodFixes #40
Alternative to / closes #64
Part of sveltejs/svelte#4694
Credit to @milahu for finding the issue, we should add a "co-authored by"-message to the merge.