dojo / core

:rocket: Dojo 2 - language helpers and utilities.
http://dojo.io
Other
213 stars 58 forks source link

Comparison API #233

Closed kitsonk closed 7 years ago

kitsonk commented 8 years ago

This PR implements a comparison API as part of core.

Fixes #138

The Comparison API can deal with Arrays and "plain" Objects. Plain objects are objects that have either Object as a constructor (or no constructor) and the properties values of the object are either primitives, arrays, or plain objects. For objects, only enumerable own properties are considered.

The module compare exports two functions diff and patch.

diff() returns the difference between the first and the second argument and describes the steps to take to make the first object look like the second object.

patch() takes a target and applies a set of changes which will change the target.

jsf-clabot commented 8 years ago

CLA assistant check
All committers have signed the CLA.

kitsonk commented 7 years ago

@maier49 @rorticus @vansimke could you please review

agubler commented 7 years ago

@maier49 can you give this a review please?

dylans commented 7 years ago

We should probably decide what to do with this... do we need to leverage more of what dojo/stores has instead, etc.? @maier49 @agubler

codecov-io commented 7 years ago

Codecov Report

Merging #233 into master will increase coverage by 0.39%. The diff coverage is 99.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   93.88%   94.28%   +0.39%     
==========================================
  Files          39       40       +1     
  Lines        2144     2309     +165     
  Branches      408      463      +55     
==========================================
+ Hits         2013     2177     +164     
  Misses         52       52              
- Partials       79       80       +1
Impacted Files Coverage Δ
src/compare.ts 99.39% <99.39%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a77d9e7...aa0e124. Read the comment docs.

maier49 commented 7 years ago

The patch implementation in stores could pretty easily be moved over to core. It only relies on a few utility functions outside of the patch package. Since it already supports serialization to JSON patch and there's a PR open to support JSON Merge Patch I'm still inclined to think that it would be better to move it over to core than to have competing implementations or switch to an implementation that doesn't yet have a plan for serialization.

We could address any issues with the API in the process of moving it over.

dylans commented 7 years ago

@maier49 ok, let's do that!

agubler commented 7 years ago

So shall we close this pending @maier49's work to move the implementation over from stores?

kitsonk commented 7 years ago

I have refreshed this PR from what I have had to do in intern-helper which needed to use this API.

Also, my understanding is that @pottedmeat had a chat with @maier49 about the usage in stores for dgrid and they agreed that it might be better to take what is here and iterate on it to make it work for stores. If that is still the case, we should land this and figure out if there are any features that are needed for stores.

Also, widget-core uses a diff function for properties, which we should evaluate if this API is sufficient for that, or if there are other changes required.

maier49 commented 7 years ago

@kitsonk the main thing for stores is just providing an appropriately serialized version of a diff for a PATCH request. I'm ok with stores using its own implementation until we figure that out here.

kitsonk commented 7 years ago

I will take a look at what is in stores (and sorry again for not looking first) and see what it would take to do that.