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 #978

Closed
mgcrea opened this issue Feb 19, 2015 · 12 comments
Closed

RangeError: Maximum call stack size exceeded #978

mgcrea opened this issue Feb 19, 2015 · 12 comments
Labels

Comments

@mgcrea
Copy link

mgcrea commented Feb 19, 2015

Probably not a bug but I got some weird error after upgrading from ~v2.4, haven't really dug into yet but it might help anyone encountering this suddenly:

Using a "standard" _.defaultsDeep like this (eg. like merge-defaults):

var defaultsDeep = _.partialRight(_.merge, function deep(value, other) {
  if (_.isArray(value) || _.isDate(other)) return value;
  return _.merge(value, other, deep);
});

This is now crashing with RangeError: Maximum call stack size exceeded:

defaultsDeep('{,modules/*/}{views,scripts,modules}/**/*.{jade,html}', '{views,modules/*}/**/*.{html,jade}');

Somehow the _.merge gets stuck on merging the { char, Haven't found something that could explain this by reading the changelog yet.

Worked in the v2.4 branch.

@jdalton
Copy link
Member

jdalton commented Feb 19, 2015

Yap, many recursive methods are susceptible to call stack limits which vary for enviros and func too size. It's not a common enough problem to address at the moment (would mean large perf hits for an edge issue)

@jdalton jdalton closed this as completed Feb 19, 2015
@jdalton
Copy link
Member

jdalton commented Feb 19, 2015

@mgcrea can you gist the object your iterating over.

@mgcrea
Copy link
Author

mgcrea commented Feb 19, 2015

@jdalton https://gist.github.com/mgcrea/523ef0ea0b37ee9b330b

Fixed it with this for now, looks like to be working:

var defaultsDeep = _.partialRight(_.merge, function deep(value, other) {
  if (_.isString(value) || _.isArray(value) || _.isDate(other)) return value;
  return _.merge(value, other, deep);
});

[edit] Might be more reusable as JSONs https://gist.github.com/mgcrea/5881666f7c71280b758c

@jdalton
Copy link
Member

jdalton commented Feb 19, 2015

These objects are tiiiiny. I'll dig in this evening.

@mgcrea
Copy link
Author

mgcrea commented Feb 19, 2015

@jdalton

Indeed, the base test-case does crash and is only merging two strings:

defaultsDeep('{,modules/*/}{views,scripts,modules}/**/*.{jade,html}', '{views,modules/*}/**/*.{html,jade}');

Might help:

$ node --version
v0.10.35
$ uname -a
Darwin 0x6D67 14.1.0 Darwin Kernel Version 14.1.0: Mon Dec 22 23:10:38 PST 2014; root:xnu-2782.10.72~2/RELEASE_X86_64 x86_64 i386 MacBookPro11,3 Darwin

@jdalton
Copy link
Member

jdalton commented Feb 19, 2015

The method isn't intended for strings so I think its probably iterating them as a string object (so iterating the index proprerties of both strings)

@jdalton jdalton added bug and removed invalid labels Feb 19, 2015
@jdalton
Copy link
Member

jdalton commented Feb 19, 2015

This is a bug on _.merge handling of string values.

@jdalton
Copy link
Member

jdalton commented Feb 19, 2015

Added tests 89ed40e.

@recurrence
Copy link

Ran into this as well with a defaultsDeep implementation when upgrading from 2.4.1 to 3.2.

@jdalton
Copy link
Member

jdalton commented Feb 20, 2015

Expect a bump, 3.3.0, by Friday evening.

@recurrence
Copy link

Thanks!

@lock
Copy link

lock bot commented Jan 20, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants