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

RangeError: Maximum call stack size exceeded in @angular/animations module #38551

Closed
th0r opened this issue Aug 21, 2020 · 4 comments
Closed
Labels
area: animations help wanted An issue that is suitable for a community contributor (based on its complexity/scope). needs reproduction This issue needs a reproduction in order for the team to investigate further P4 A relatively minor issue that is not relevant to core functions
Milestone

Comments

@th0r
Copy link

th0r commented Aug 21, 2020

🐞 bug report

Affected Package

The issue is caused by package @angular/animations

Is this a regression?

Not sure

Description

We sometimes see RangeError: Maximum call stack size exceeded error in our production environment. Some of them come from Safari, some from Chrome. I'm attaching a few last frames of the stacktrace (see the section below). I think this is the reason for the issue: https://anchortagdev.com/range-error-maximum-call-stack-size-exceeded-error-using-spread-operator-in-node-js-javascript. Looks like it happens when page contains a lot of DOM elements and code tries to push them all into an array using a spread operator.

🔬 Minimal Reproduction

Sorry, I don't know how to reproduce it.

🔥 Exception or Error

image

🌍 Your Environment

Angular Version:




Angular CLI: 9.1.5
Node: 12.16.1
OS: darwin x64

Angular: 9.1.6
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router, upgrade
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.5
@angular-devkit/build-angular     0.901.5
@angular-devkit/build-optimizer   0.901.5
@angular-devkit/build-webpack     0.901.5
@angular-devkit/core              7.3.10
@angular-devkit/schematics        7.3.10
@angular/cdk                      9.2.3
@angular/cli                      9.1.5
@ngtools/webpack                  9.1.5
@schematics/angular               9.1.5
@schematics/update                0.901.5
rxjs                              6.5.5
typescript                        3.7.5
webpack                           4.43.0
@ngbot ngbot bot added this to the needsTriage milestone Aug 21, 2020
@th0r th0r changed the title RangeError: Maximum call stack size exceeded in @angular/animations module in Safari RangeError: Maximum call stack size exceeded in @angular/animations module Sep 3, 2020
@th0r
Copy link
Author

th0r commented Sep 15, 2020

Any feedback folks?

@AleksanderBodurri
Copy link
Member

This issue got me really curious so I did some benchmarking with https://jsbench.me

https://jsbench.me/jvkf8ynmgg/1

Each test creates 1000 divs before the test case runs and assigns the result to a variable.

  1. nodeList kept as a NodeList

Screen Shot 2020-09-18 at 9 07 20 PM

  1. nodeList converted into an Array (This one is unrelated to this issue but I thought it was interesting)

Screen Shot 2020-09-18 at 9 08 13 PM

Relevant code

_query = (element: any, selector: string, multi: boolean): any[] => {
let results: any[] = [];
if (multi) {
results.push(...element.querySelectorAll(selector));
} else {
const elm = element.querySelector(selector);
if (elm) {
results.push(elm);
}
}
return results;
};

Maybe changing

     results.push(...element.querySelectorAll(selector)); 

to

    const elements = element.querySelectorAll(selector);
    for (let i = 0, l = elements.length; i < l; i++) {
       results.push(elements[i]);
    }

might resolve this issue and potentially offer a performance increase.

@jelbourn jelbourn added help wanted An issue that is suitable for a community contributor (based on its complexity/scope). P4 A relatively minor issue that is not relevant to core functions labels Oct 26, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Oct 26, 2020
@jelbourn
Copy link
Member

While we can't reproduce this issue, it looks like the function @AleksanderBodurri mentions could be simplified anyway to something like this:

_query = (element: any, selector: string, multi: boolean): any[] => {
    if (multi) {
        return Array.from(element.querySelectorAll(selector));
    } else {
      const elm = element.querySelector(selector);
      return elm ? [elm] : [];  
    }
  };

Feel free to send a PR

@jelbourn jelbourn added the needs reproduction This issue needs a reproduction in order for the team to investigate further label Oct 26, 2020
basherr added a commit to basherr/angular that referenced this issue Nov 25, 2020
…d" to "for"

For element queries that return sufficiently large NodeList objects, using spread syntax to populate the results array causes a RangeError due to the call stack limit being reached. This commit updates the code to use regular "for" loop instead.

Fixes angular#38551.
basherr added a commit to basherr/angular that referenced this issue Nov 25, 2020
…d" to "for"

For element queries that return sufficiently large NodeList objects, using spread syntax to populate the results array causes a RangeError due to the call stack limit being reached. This commit updates the code to use regular "for" loop instead.

Fixes angular#38551.
basherr added a commit to basherr/angular that referenced this issue Nov 25, 2020
…d" to "for"

For element queries that return sufficiently large NodeList
objects, using spread syntax to populate the results array
causes a RangeError due to the call stack limit being reached.
This commit updates the code to use regular "for" loop instead.

Fixes angular#38551.
jessicajaniuk pushed a commit that referenced this issue Nov 25, 2020
…d" to "for" (#39646)

For element queries that return sufficiently large NodeList
objects, using spread syntax to populate the results array
causes a RangeError due to the call stack limit being reached.
This commit updates the code to use regular "for" loop instead.

Fixes #38551.

PR Close #39646
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: animations help wanted An issue that is suitable for a community contributor (based on its complexity/scope). needs reproduction This issue needs a reproduction in order for the team to investigate further P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
4 participants