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

Improve on jk patch #128

Closed
dlespiau opened this issue Feb 13, 2019 · 6 comments · Fixed by #245
Closed

Improve on jk patch #128

dlespiau opened this issue Feb 13, 2019 · 6 comments · Fixed by #245
Labels
kind/enhancement New feature or request question Further information is requested topic/std Relates to the standard library
Milestone

Comments

@dlespiau
Copy link
Member

dlespiau commented Feb 13, 2019

Currently, patch will always "deep merge" objects, ie, given:

const a = {
   key: {
     n: 2,
     s: "aString",
   },
};
patch(a, { key: { s: "anotherString" } });

will give:

{
   key: {
     n: 2,
     s: "anotherString",
   },
};

We could support an alternative merging strategy, which is to replace the value instead of deep merging it:

patch(a, { key: { s: "anotherString" } });

will give:

{
   key: {
     s: "anotherString",
   },
};

Jsonnet let people the choice of the strategy. By default, it will override the value but, by appending a +: operator to the key, the user can signal they want a deep merge instead (look for "+:" in the page: https://jsonnet.org/learning/tutorial.html)

We could support something similar, even borrowing the syntax:

patch(a, { key: { s: "anotherString" } });
->
{
   key: {
     s: "anotherString",
   },
};
patch(a, { 'key+': { s: "anotherString" } });
->
{
   key: {
     n: 2,
     s: "anotherString",
   },
};

We could also make the '+' work for arrays and say, concatenate arrays of replacing them.

@dlespiau dlespiau added kind/enhancement New feature or request question Further information is requested labels Feb 13, 2019
@squaremo
Copy link
Member

This is nice for merging a literal into another value ✔️

I think we will always need a procedure which will merge two values that came from computation or a file, since going through and annotating such a thing would be painful. (It doesn't have to unconditionally do deep-merge).

@dlespiau
Copy link
Member Author

Other things we talked about:

  • Have Symbols attached to an object describing merge strategies for that specific object
  • Have a separate list of merging strategies/operations next to an object.

@dlespiau dlespiau added the topic/std Relates to the standard library label Feb 17, 2019
@dlespiau dlespiau added this to the 0.3.0 milestone Mar 1, 2019
@rndstr
Copy link

rndstr commented Mar 28, 2019

Just wanted to put some other known merge/patch strategies on the table:

@dlespiau
Copy link
Member Author

dlespiau commented Mar 28, 2019

Also the merge behaviour in Hiera

@dlespiau
Copy link
Member Author

dlespiau commented Apr 14, 2019

There are many merging strategies, eg.:

  • For objects:
    • replace the orignal object by the patch object: strategy could be named assign or replace.
    • deep merge it with the orignal value: deep
    • Ramda's mergeDeepWith and mergeDeepWithKey are a nice escape for people to define extra stratgies
  • For arrays:
    • assign
    • concatenate the second one to the first one
    • final array is sorted with both sources elements
    • only keep unique array items

At the lower level, we should be able to configure the merging strategy for specific paths in objects. So it would look like:

const a = {
   key: {
     n: 2,
     s: "aString",
   },
};

const rules = {
  '.': deep,
  'key': assign:,
}

patch(a, b, rules)

If unspecified, we need a default: deep for objects (?), assign for primitive types, not sure what should be the default for arrays (assign?).

I think it'd be nice of the right hand side of rules are functions so people can define their own. This way they could define special merging strategies, even for primitive types! For instance if two objects have a count field, it could make sense that the resulting merged object has a count field with is the sum of the two counts.

Once we have that low level representation and if we want to support jsonnet patches, it should be straightforward to take a jsonnet patch and compile down a set of rules from it.

@dlespiau
Copy link
Member Author

Another source of inspiration: https://reclass.pantsfullofunix.net/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request question Further information is requested topic/std Relates to the standard library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants