Skip to content
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

feat: autofix error "Maximum call stack size exceeded" in array push spread #78

Closed
milahu opened this issue Sep 1, 2021 · 4 comments
Closed
Labels
question Further information is requested

Comments

@milahu
Copy link

milahu commented Sep 1, 2021

calling someArray.push(...otherArray) can be considered a code smell,
since otherArray.length is limited to about 100K in nodejs (500K in firefox)

// problem
[].push(...Array.from({ length: 1000*1000 })) // ... = spread operator
// Uncaught RangeError: Maximum call stack size exceeded

// solution
var input = Array.from({ length: 1000*1000 }); var output = [];
for (let i = 0; i < input.length; i++) output.push(input[i]);
output.length
// 1000000

// solution
Array.prototype._concat_inplace = function(other) { // aka _push_array
  for (let i = 0; i < other.length; i++) {
    this.push(other[i]);
  }
  return this; // chainable
};
var input = Array.from({ length: 1000*1000 }); var output = [];
output._concat_inplace(input), output.length;
// 1000000

see sveltejs/svelte#4694 (comment)

@coderaiser
Copy link
Owner

It definitely can, every problem has a solution, but not every solution relates to a problem :).

Could you please provide me a fixture so I can reproduce this. But remember that such code should be something that people work with. Putout makes code better, but such code should be written in a way developers does.

Using for with iterator requires more cognitive load and should be moved out to "hot" function that works with such amount of data. When we talk about rules count in putout for example it wan't be 100 * 100, this is too much information to put in one head.

About prototypes. We had prototypes.js example, and colors.js which was displaced with chalk. This is bad practice to put something in prototype so I can't recommend such thing to anybody.

Anyways example you show very important and people should know about it and use according to Pareto principle, to 20% of their code they sure that will have so much data.

@coderaiser coderaiser added the question Further information is requested label Sep 1, 2021
@milahu
Copy link
Author

milahu commented Sep 1, 2021

This is bad practice to put something in prototype so I can't recommend such thing to anybody.

but its the shortest solution, "written in a way developers does"

ideally a future version of node would implement something like Array.prototype.pushArray
so that this works

[].pushArray(Array.from({ length: 1000*1000 }))

also, this rule would be default off, and only would be used to transform code which has this problem

or move to eslint?

@coderaiser
Copy link
Owner

I think would be great if you write about it to https://es.discourse.group/ and ESLint, this is very interesting what @nzakas thinks about it, he may like this idea, because it makes code more stable.

@milahu
Copy link
Author

milahu commented Sep 6, 2021

working prototype at
https://github.com/milahu/random/tree/master/svelte/patch-svelte-compiler-sources

basically, ive reinvented putout and eslint ; )
not sure if these tools are powerful enough to handle this transform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants