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

Consider adding array merge strategy option #31

Open
webketje opened this issue Mar 10, 2022 · 1 comment
Open

Consider adding array merge strategy option #31

webketje opened this issue Mar 10, 2022 · 1 comment

Comments

@webketje
Copy link

webketje commented Mar 10, 2022

Given following code:

const host = { arr: ['hello'] }
const { dset } = require('dset/merge')
dset(host, 'arr', ['world'])

// or
const host = { arr: ['hello'] }
const { merge } = require('dset/merge')
merge({arr: ['hello']}, {arr: ['world']})

I would expect the result to be { arr: ['hello', 'world'] }, however currently the result is { arr: ['world'] }
If I change the second arr to barr the result is { arr: ['hello'], barr: ['world'] }

The README says:

The main/default dset module forcibly writes values at the assigned key-path. However, in some cases, you may prefer to merge values at the key-path

So I would expect arrays to merge too instead of being forcibly overwritten as in the 'standard' dset
Anyhow i understand the merge logic is similar to lodash.merge.
The deepmerge module has a merge strategy option, but it only handles merging, not what dset does.
It would be great if dset could be used with a strategy of "add" instead of "overwrite".

+ strat = strat || (a, b, k) => { a[k] = merge(a[k], b[k]) };
if (Array.isArray(a) && Array.isArray(b)) {
  for (k=0; k < b.length; k++) {
-    a[k] = merge(a[k], b[k]);
+   strat(a, b, k)
  }
}
+ // alt strat: (a, b, k) => a.push(b[k])
@paulovieira
Copy link

This was also my expectation about dset/merge when dealing with arrays. But thinking a bit about this, it's not so clear what is meant by "merging" in the context of arrays. Usually the new elements should be placed at the end, but sometimes might be at the beginning.

Anyway, it's easy to create a wrapper around dset/merge to handle this case:

import delve from 'dlv';
import { dset as dsetMergeOriginal } from 'dset/merge';

function dset(obj, keyPath, val, arrayMethod = 'push') {
  let nestedValue = delve(obj, keyPath);

  // manually handle merging of arrays ('dset/merge' will only merge objects)

  if (Array.isArray(nestedValue) && Array.isArray(val)) {
    for (let i = 0; i < val.length; i++) {
      nestedValue[arrayMethod](val[i]);
    }
  }
  else {
    dsetMergeOriginal(obj, keyPath, val); 
  }
}

export {
  dset
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants