Add support for Node.js 12.x#1728
Conversation
|
Any objections against back-porting this change to 3.x? |
| self._coerce(clauses[k], options); | ||
| } | ||
|
|
||
| where[p] = clauses; |
There was a problem hiding this comment.
This is kind of weird. Before my change, the block "handle logical operators" did not apply updates made by coerceArray to the original where[p] value. As a result, array-like object {1: {field: 'value'}} was not converted into an array [{field: 'value'}]. I am not sure why this was not caught earlier and surfaced only with Node.js 12. Is it perhaps a change in how deep-equality checks work on the new Node version? IDK.
There was a problem hiding this comment.
Here are the tests that are failing without my fix:
- Fix code to pass all tests on Node.js 12 - Add Node.js 12 to the list of Travis CI platforms
7ef195c to
5c3405e
Compare
b-admike
left a comment
b-admike
left a comment
There was a problem hiding this comment.
LGTM, I think the changes you made in order to add support are reasonable.
jannyHou
left a comment
jannyHou
left a comment
There was a problem hiding this comment.
👍 LGTM when jenkins tests pass.
|
@slnode test please |
1 similar comment
|
@slnode test please |
4 participants
Related issues
Checklist
guide