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

Deep put #151

Open
srph opened this issue Aug 25, 2016 · 11 comments
Open

Deep put #151

srph opened this issue Aug 25, 2016 · 11 comments

Comments

@srph
Copy link
Contributor

srph commented Aug 25, 2016

Currently put [immutably] only sets the value to the object path. I was wondering if it would be possible to merge the new object to the current.

Sample Data

var user = {
  name: 'Brian Mae S. Bodollo',
  mother: {
    name: 'Marian Rivera',

    mother: {
      name: 'Claire Borromeo',

      daughters: [{
        name: 'Marian Rivera' 
      }]
    }
  },
};

Current

var updated = put(user, {
  'mother.mother.daughters.0.age': 5,
  'mother.mother.daughters.0.height': 6,
  'mother.mother.daughters.0.width': 6
});

Suggestion:

var updated = put(user, 'mother.mother.daughters.0', {
  age: 5,
  height: 6,
  width: 6
});

Maybe we could have a new method (named update) instead of updating the functionality of put?

Let me know if I missed out anything. Thanks!

@tjmehta
Copy link
Owner

tjmehta commented Aug 25, 2016

Quick reply for now. I recently fixed a bug with put. I think the example you provided should work as expected with the latest version of 101.

Try it out and see if the problem remains:

'npm i --save 101@latest'

Lmk,
Typed using my thumbs..

On Aug 25, 2016, at 2:24 PM, Kier Borromeo [email protected] wrote:

Currently put [immutably] only sets the value to the object path. I was wondering if it would be possible to merge the new object to the current.

Sample Data

var user = {
name: 'Brian Mae S. Bodollo',
mother: {
name: 'Marian Rivera',

mother: {
  name: 'Claire Borromeo',

  daughters: [{
    name: 'Marian Rivera' 
  }]
}

},
};
Current

var updated = put(user, {
'mother.mother.daughters.0.age': 5,
'mother.mother.daughters.0.height': 6,
'mother.mother.daughters.0.width': 6
});
Suggestion:

var updated = put(user, 'mother.mother.daughters.0', {
age: 5,
height: 6,
width: 6
});
Maybe we could have a new method (named update) instead of updating the functionality of put?

Let me know if I missed out anything. Thanks!


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@srph
Copy link
Contributor Author

srph commented Aug 26, 2016

Thanks for the quick reply! <3

Yea, I'm using the latest version. It doesn't seem to be fixed 😕 (Demo).

For clarification, the end result (to the object mentioned previously) should be:

console.log(user.mother.mother.daughters[0], updated.mother.mother.daughters[0]);
// { name: 'Immaculate Borromeo' }
// { name: 'Immaculate Borromeo', age: 5, height: 6, width: 6 }

Moreover, I didn't find any test case about this scenario.

Let me know where I can help :)

@tjmehta
Copy link
Owner

tjmehta commented Aug 26, 2016

I see what you are saying now. Sorry, looked at that too quick on my phone.
Let me think about this a bit. Would keypather.flatten be sufficient for your needs?:

var keypather = require('keypather')()
var put = require('101/put')

var update = keypather.flatten({
  age: 5,
  height: 6,
  width: 6
}, '.', 'mother.mother.daughters.0')
put(user, update)

@srph
Copy link
Contributor Author

srph commented Aug 26, 2016

Yea. But to simplify even more, I was wondering if we could achieve the ff:

var update = require('101/update');

var updated = update(user, 'mother.mother.daughters.0', {
  age: 5,
  height: 6,
  width: 6
});

Similar to put but merges the object :D

@tjmehta
Copy link
Owner

tjmehta commented Aug 26, 2016

Could we extend '101/assign'?

@tjmehta
Copy link
Owner

tjmehta commented Aug 26, 2016

Assign current only supports a depth of 1

@tjmehta
Copy link
Owner

tjmehta commented Aug 26, 2016

And doesn't support keypaths

@srph
Copy link
Contributor Author

srph commented Aug 27, 2016

I think we could do that, but I think it could complicate assign in both functionality and API. IMO, I would prefer a new function like update :D.

@tjmehta
Copy link
Owner

tjmehta commented Aug 27, 2016

The plan for assign is to possibly deprecate it in v2.0.0.

Actually to get this in before then how about '101/extend'. I am traveling this week, but happy to take a PR.

Typed using my thumbs..

On Aug 26, 2016, at 10:26 PM, Kier Borromeo [email protected] wrote:

I think we could do that, but I think it could complicate assign in both functionality and API. IMO, I would prefer a new function like update :D.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@srph
Copy link
Contributor Author

srph commented Aug 27, 2016

Oh, how about we name it merge? _.merge is like _.assign but recursively merges the object. However, _.merge mutates the original object.

@tjmehta
Copy link
Owner

tjmehta commented Aug 27, 2016

👍

Typed using my thumbs..

On Aug 27, 2016, at 1:25 AM, Kier Borromeo [email protected] wrote:

Oh, how about we name it merge? _.merge is like _.assign but recursively merges the object.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@srph srph mentioned this issue Apr 29, 2017
6 tasks
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