Serialize context JSON deterministically #414
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clehner
requested review from
OR13,
mprorock,
mkhraisha and
nissimsan
as code owners
nissimsan
approved these changes
May 25, 2022
OR13
approved these changes
May 27, 2022
mkhraisha
approved these changes
May 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A recent change in the constructed/published context file consisted solely of reordering of properties:
17060ea#diff-90421741a2d21ee4772ed49f297a37c7e6ac8e0c8b28bf1bae5159816642787e
17060ea from cf24c12 (#405).
Order of the properties does not affect the meaning in JSON-LD. It was not immediately obvious to me when examining the change that there were no actual/substantive changes - until passing both the old and new version through a deterministic serialization.
Diffing the changed context file shows a large diff:
But after piping the old and new version through
jq -Sto sort the properties, the resulting diff is empty:Minimizing changes that have no effect helps make diffs to the context files more readable to see what actual changes are taking place.
This PR proposes to deterministically serialize the context file serialized using json-stringify-deterministic, rather than using
JSON.stringifywhich is not deterministic. This should help make future changes to the generated context file more readable in the commit history and diffs.This is not a JSON-LD-significant serialization change; it is only for reducing variance in the JSON serialization, which is insignificant to JSON-LD processing (and to JSON parsing if your JSON implementation does not preserve property order).