throw error in mergeWith method if no merger is passed (#1503) #1543
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
@facebook-github-bot Just signed CLA |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
src/functional/merge.js
Outdated
| @@ -38,7 +38,8 @@ export function mergeWithSources(collection, sources, merger) { | |||
| ); | |||
| } | |||
| if (isImmutable(collection)) { | |||
| return collection.mergeWith | |||
| const shouldMergeWith = collection.mergeWith && merger instanceof Function; | |||
There was a problem hiding this comment.
This logic translates all calls to Immutable.merge(collection, sources) into calls to collection.concat(...sources). Should instead check for collection.merge if merger isn't a function. Something like:
if (collection.mergeWith && merger instanceOf Function) {
return collection.mergeWith(merger, ...sources);
} else if (collection.merge && !(merger instanceOf Function) { // REALLY not sure about this condition
return collection.merge(...sources);
} else {
return collection.concat(...sources);
}
|
@asazernik thanks for taking a peak, pushed a commit. |
Fixes #1503 Squashed commit of the following: commit a5091619058844f6bfc758f9111a34e17d72bc09 Author: Lee Byron <lee.byron@robinhood.com> Date: Tue Sep 18 11:59:46 2018 -0700 Refinements commit 4d04397435ec323bd9f8e598b651acf45ee683fe Merge: 37ae5be df8b50a Author: Lee Byron <lee.byron@robinhood.com> Date: Tue Sep 18 11:46:51 2018 -0700 Merge branch 'master' of https://github.com/Brantron/immutable-js into Brantron-master commit df8b50a Author: Brandon Lawrence <brandon.lawrence@contactually.com> Date: Sun Jul 15 19:48:19 2018 -0400 merge vs concat when possible in functional/merge commit 2916161 Author: Brandon Lawrence <brandon.lawrence@contactually.com> Date: Fri Jun 15 07:39:22 2018 -0400 throw error in mergeWith method if no merger is passed (#1503)
|
Squash merged in 2ea44e0, thanks! |
This ensures users get an informative error when calling
mergeWithwithout a merge functionDetails can be found at #1503.