Skip to content

Conversation

ericbf
Copy link
Contributor

@ericbf ericbf commented Mar 21, 2023

With a comparator function, there’s not a hard requirement for the two arrays to be compatible types. The comparator tells us whether they are “equal”, and that’s enough. This will maintain the integrity between which type corresponds to what in the comparators, as well as the functions where appropriate.

This doesn’t touch getPatch or applyPatch, because patching doesn’t make sense if the types don’t match.

Related to #90

With a comparator function, there's not a hard requirement for the two arrays to be compatible types. The comparator tells us whether they are "equal", and that's enough. This will maintain
the integrity between which type corresponds to what in the comparators, as well as the functions where appropriate.

This doesn't touch `getPatch` or `applyPatch`, because patching doesn’t make sense if the types don’t match.
@ericbf
Copy link
Contributor Author

ericbf commented Mar 21, 2023

I’m doing some work where I need to find the diff between a list of documents and a list of corresponding draft documents, which have different types in enough ways that it gets tedious to try to combine them into a shared interface. As stated in the description above, the functions that do the heavy lifting don’t actually require that the type between the two arrays be the same. This PR expands the types to allow that (while still providing a default for the second array's type so that the user could still provide a single type param if desired, making it a non-breaking change).

Let me know what you think.

@coveralls
Copy link

coveralls commented Mar 21, 2023

Coverage Status

Coverage: 100.0%. Remained the same when pulling 395453b on ericbf:differently-typed-arrays into 625d5f0 on YuJianrong:master.

@YuJianrong
Copy link
Owner

@ericbf This is an excellent idea to make the diff support for different types of comparison!
Can you add a few test cases under the src/test folder to cover a few cases of this code? I want to add it, but I found the code is on your repo. If you don't want to add the test cases, I can merge this PR and add it later.

@ericbf
Copy link
Contributor Author

ericbf commented Mar 21, 2023

I can definitely add some test cases

@YuJianrong
Copy link
Owner

@ericbf Thanks very much for your contrbution! Everything looks perfect!

@YuJianrong YuJianrong merged commit 9da10de into YuJianrong:master Mar 21, 2023
github-actions bot pushed a commit that referenced this pull request Mar 21, 2023
# [1.1.0](v1.0.1...v1.1.0) (2023-03-21)

### Features

* expand types to allow differently typed arrays ([#156](#156)) ([9da10de](9da10de))
@github-actions
Copy link

🎉 This PR is included in version 1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Mar 21, 2023
# [1.1.0](v1.0.1...v1.1.0) (2023-03-21)

### Features

* expand types to allow differently typed arrays ([#156](#156)) ([9da10de](9da10de))
@ericbf ericbf deleted the differently-typed-arrays branch March 21, 2023 20:59
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

Successfully merging this pull request may close these issues.

3 participants