-
Notifications
You must be signed in to change notification settings - Fork 16
Modernize and rewrite into TypeScript #85
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
Draft
novemberborn
wants to merge
64
commits into
main
Choose a base branch
from
rewrite
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
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
Closed
This was
linked to
issues
Apr 12, 2025
Open
Open
Refactor how values are described and (de)serialized to be more extensible. This comes with possible subtle changes in behavior: * Well-known symbols are now detected in the environment * Deserialized representations can be used on either side of a comparison * Arguments objects can still be compared against an array (but not vice versa) * Handle Node.js-specific External objects * Handle Web CryptoKeys * Handle module namespace objects (which have live bindings) * Handle weak maps and sets * Handle `maxByteLength` and `resizable` properties of ArrayBuffers, and `maxLength` and `growable` properties of SharedArrayBuffers * Handle empty slots in sparse arrays * Handle `cause` property of Errors; `name`, `message` and `cause` are always recognized even if not enumerable Representations are encoded using CBOR types. The plugin infrastructure has been removed for now, since there are no actual plugins available anyway.
Fix accidental iteration over string characters.
Frequently occuring values should have predictable prefixes that can be used in a compression dictionary.
…constructor name Store these as the true type, and assign the constructor name when deserializing. This saves us from encoding the constructor name twice for a single value.
…pes an opt-in behavior
Stop using interfaces, where possible implement a type directly. Improve separation of functionality between shallow and deep representations. For value representations, implement multiple types while also declaring union types for consuming functions. Use a 'satisfies' construct to confirm within the module that the implementation is type-safe.
Don't return undefined, like we do if the constructor is not a function.
They can't actually be serialized or formatted.
* 'Descriptor' is now 'representation' * Thus, 'describe' becomes 'representValue' * And 'description context' becomes 'real value context' to separate from 'deserialization context'
This does require us to know whether a representation was deserialized to begin with.
Rely on fullyDeserialize() to proactively deserialize the property value. The value no longer needs to be encoded as a uint8array, which was required so a single read (of the uint8array) would progress the decoder to the next property. The downside is that deeply nested property values are fully deserialized, which could be unnecessary when an object has a single symbol property and is compared against another object with a different symbol property. However in more typical scenarios this shouldn't have a measurable performance impact, if only because if values are equal we'll eventually fully deserialize anyway.
…holds (And the stack isn't modified.)
Having a generic notion of 'grouped accessors' means we can implement different comparison logic, e.g. partial and unordered, even for maps and sets. We can also remove the complicated deserialization logic to yield groups.
…toKey Always require the other side to be a similar representation; don't return 'possibly equal' when we want properties to be compared. And remove other misleading, duplicative tests.
And make it possible to indeed exclude properties.
Even if not enumerable.
* Update compare algorithm to handle unbalanced trees * Allow array-like to be compared against arrays; and arrays against arrays; in both cases where RHS is possibly shorter. This means we have a prefix-comparison.
"Array-like" now means "like an array" without being an actual array. * Define a getter on ObjectRepresentation that checks whether the object is array-like * Override in subclasses for values that have a length property but are not array-like * Rename `iterateArrayLike` to `iterateElements` for clarity * Remove unnecessary overrides and tests for `iterateElements` * Change annotation logic to allow `l` to be provided, destructure other known annotations and explicitly order them
Most complex values extend `ObjectRepresentation`. If LHS in a comparison is an `ObjectRepresentation`, it could be compared against any other subclass. Normally constructor names won't match, but that's not the case for fuzzy matching. This adds a new `acceptsComparisonFrom` method to all value represenations. It enables RHS to control whether it can be compared to LHS. Subclasses are expected to override to avoid unintentional comparisons from their parent class. This also allows us to invert control of certain exceptional comparison scenarios, such as whether an arguments object can be compared to an array, or a sparse array value to `undefined`, plus a host of fuzzy matching behaviors. Consequently we can also remove most static `is()` methods, since the various representation classes don't need to reference each other.
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.
Rewrite the implementation in TypeScript, using modern syntax. Refactor how values are described and (de)serialized to be more extensible. Possible subtle changes in behavior. Detect well-known symbols. Encode using CBOR types. Remove plugin infrastructure for now, since there are no actual plugins available anyway.