Fix parsing & compacting very deep objects #224
Conversation
lib/utils.js
Outdated
| return exports.compactQueue(queue); | ||
| }; | ||
|
|
||
| exports.compactQueue = function (queue) { |
There was a problem hiding this comment.
does this need to be publicly exported? It'd be nice not to increase the public API.
There was a problem hiding this comment.
It doesn't, but didn't see the utils object being exported out through the index entry point. I wasn't sure if adding to exports was just the style or not (most of it was changed to the Hapi project style when it lived there for a bit and I just wasn't super familiar).
TL;DR thought that it would fit in that way, but I'll in-export the function 👍
There was a problem hiding this comment.
It's certainly consistent :-) but I'm in a slow yet determined march to move away from the hapi style, and minimize the publicly reachable API of qs, so the change is appreciated!
There was a problem hiding this comment.
haha, making the change is no problem at all :) I try to copy the existing code style as much as possible to blend it in, so wasn't sure, hehe
ljharb
added
parse
semver-minor: new features
semver-patch: bugfix/patch/etc
and removed
semver-minor: new features
labels
|
Looks like your new test is only failing in Turbofan-enabled nodes - 8.3+. |
|
Yep, just saw that and taking a look :) |
|
Ok, for whatever reason Turbofan was just failing to collapse in the same function Crankshaft was doing in the actual parsing. So now parsing itself is iterative instead of recursive. Probably even a better win overall now for everyone. |
dougwilson
changed the title
|
Released in v6.5.1 |
This is a change of the internal
compactutility to use an iterative approach instead of recursion in order to better handle very, very deep objects without a stack overflow. It was noticed on CharkaCore because it has a much smaller maximum stack than v8 does, though it can still happen on v8 (which the test does)./cc @kunalspathak