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

feat: use IDs in arrays for more semantic version diff #11671

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

philipp-tailor
Copy link

What?

For arrays, where every entry has an id, use the id to track the movement of elements when computing the version diff. This means that e.g. deleting an entry in the middle, or adding an entry in the middle no longer causes all following array elements to be shown as fully changed, making the version diff easier to read for humans (see screenshots below for how diffs would render now).

Arrays where (some) entries lack the id are compared as before, by index.

Version diff in array where elements have id when a new entry is created in 1st place:

image
status quo image

Version diff in array where elements have id when the first entry is deleted:

image
status quo image

Version diff in array where elements have id when a new entry is created in the middle:

image
status quo image

Version diff in array where elements have id when an entry is deleted in the middle:

image
status quo image

Version diff in array where elements have id when an an entry is added at the end:

image

Version diff in array where elements have id when an entry is deleted at the end:

image

Why?

Makes version diff easier to parse for humans, because only changed parts are highlighted.

Possible Follow-Ups

This change is an improvement, but the diff could be even more human readable: Right now, when an array element is removed, the version diff only shows properties that were set as removed, not the whole element. Likewise, when an element is moved to a different place in the array, there's no indicator that the element was merely moved, not 1 element removed, and 1 element added. So the "labelling" of rows in the version diff could still be more semantic.

How?

packages/next/src/views/Version/RenderFieldsToDiff/buildVersionFields.tsx checks whether all elements of an array field, both in the comparison and the new version contain an id property. If that's not the case, the current, index-based code path is used. But if it is the case, the new utility packages/next/src/views/Version/RenderFieldsToDiff/utilities/getArrayElementIdsInComparisonOrder.ts is called to get the union of all array element IDs in a reasonable order, which is used to iteratively call buildVersionFields with the comparisonSiblingData of the array element with the matching id property (if existing).

For arrays, where every entry has an `id`, use the `id` to track the
movement of elements when computing the version diff. This means that
e.g. deleting an entry in the middle, or adding an entry in the middle
no longer causes all following array elements to be shown as fully
changed, making the version diff easier to read for humans.

Arrays where (some) entries lack the `id` are compared as before, by
index.
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.

1 participant