Be more strict and less custom - #655
Merged
Merged
Conversation
Owner
Author
|
This PR also drops the second The new behaviour corresponds to having this argument set I'm going to merge this to unblock further changes. It's pretty likely that this will mean that a number of other PRs will need to be updated to accommodate these changes. |
This file contains hidden or 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
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.
This PR includes a bunch of breaking changes for v3.
The general thrust here is ensure/enforce that values within a Document are wrapped in Node values, and to remove the custom instance checks. By only releasing the package as an ESM module, we avoid any dual package hazard, and so can simply rely on native
isinstancechecks.With the changes here, when a value is added using e.g.
doc.add()or.addIn(), the values are automatically wrapped/represented using appropriate Node instances. Node creation is also refactored, but that should be an implementation detail.The root value of a document is now available as its
.value(previously.contents), and it is no longer allowed to benull, but must instead be a Node. Similarly, all mapping keys (i.e.pair.keyvalues) must be Nodes, and cannot benull. Mapping values (pair.value) can still benull, or a Node value.Internally, the Node-ness of Document values is no longer checked, but is assumed. So assigning non-Node values to
doc.valueor to collection.itemsis likely to cause misbehaviour.