fix: fixed error finding refs in an object with a length property

[argos83]

Hi! we bumped into this error (our library depends on a third-party library that depends on json-refs).

json-refs fail to traverse through an object that contains a length property, because of a wrong usage of lodash (see this).

We found the bug when objects like the following took a very long time to be processed:

{ "someObject": { "value": "hi", "length": 1000000} }

The issue was on this line, lodash's each on an object like someObject in the example above will invoke the callback 1000000 times with:

[undefined, 0], [undefined, 1], ..... [undefined, 999999]

instead of two times with:

["hi", "value"], ["length", 1000000]

as it assumes it is a collection because of the length property. To iterate key, values in an object forOwn should be used instead.

[argos83]

FYI, commander 2.12 breaks two of your tests as it changes the way the output looks. It wasn't a trivial thing to fix, so I've pinned it to 2.11, and let you decide how you want to fix them.

[BenSayers]

Looks good to me

[martywins]

Any update on this?

[igorsechyn]

Can we please merge this? It is causing bugs in our codebase as well. Thank you.

[mauriedo]

Hello, any news on this? We are very keen to see this one fixed. Cheers!

[whitlockjc]

Do you know of any other places where _.each is used that should be turned into a _.forOwn?

[argos83]

I did a search of .each throughout the codebase and found some others that might have the same issue, but I'm not familiarized with the code and didn't want to change them and possibly introduced a bug by doing so. Anyways, after fixing that single .each I could not longer reproduce the issue.

[BenSayers]

@whitlockjc the bug this PR is fixing is blocking my team, can we get it merged please?

[whitlockjc]

[email protected] was just released with these changes. Thanks for the PR.

[argos83]

🎉 Thanks!