Skip to content

doc: add CompareOptions.filterer#18

Open
JakobJingleheimer wants to merge 1 commit into
mainfrom
doc/add-filterer
Open

doc: add CompareOptions.filterer#18
JakobJingleheimer wants to merge 1 commit into
mainfrom
doc/add-filterer

Conversation

@JakobJingleheimer

Copy link
Copy Markdown
Member

As noted by Justin in the May plenary, this could have quite some advantages (performance, such as avoiding the needless expense of constructing an Iterator just to immediately exclude all its items).

@gibson042 gibson042 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little torn on this, but leaning towards opposition. The only advantage of passing in a predicate rather than applying it to iterator output is that it allows filtering to affect boolean output. But it comes at the cost of making that mode intertwine implementation code with user code, which almost always incurs a performance penalty. I think it's easy enough to get boolean output like const isDifferent = !!deviations.filter(filterer).next().value, and if that much ceremony is too unergonomic then we can push to fix it with a new iterator helper, e.g. const isDifferent = !deviations.filter(filterer).isEmpty().

Comment thread README.md

```ts
type CompareOptions = {
filterer?: (a: unknown, b: unknown) => boolean,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What inputs are expected for a and b? If they're values in the object graph, then how does a filterer differentiate

  • extra vs. missing vs. mismatch
  • property descriptor
  • prototype

or just in general, where in the graph its invocation relates to?

@JakobJingleheimer JakobJingleheimer Jun 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, I suppose the type of A and B are not unknown: they're a primitive (or "special" key). And a couple things are missing.

So I guess the type would be more like:

Suggested change
filterer?: (a: unknown, b: unknown) => boolean,
filterer?: (
a: Primitive,
b: Primitive,
path: (string | Symbol)[],
reason: 'extra' | 'missing' | 'mismatch',
) => boolean,

For descriptor, I think we already covered this: A and/or B would have that special key in path (which was missing before, so that probably created the confusion).

RE the "special" path segment, what about a well-known symbol, like Symbol(Deviation:Descriptor) & Symbol(Deviation:Prototype) (or Symbol(DescriptorDeviation) & Symbol(PrototypeDeviation))? I think I'd prefer the : delimited version.


Thoughts on mismatchunequal, equality, or maybe even the equality algo used like same-zero? mismatch seems a little "shit happened" to me 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE the "special" path segment, what about a well-known symbol, like Symbol(Deviation:Descriptor) & Symbol(Deviation:Prototype) (or Symbol(DescriptorDeviation) & Symbol(PrototypeDeviation))? I think I'd prefer the : delimited version.

The special path segment can't be a symbol, because any such symbol could itself be an actual property key—e.g., { foo: { [Symbol(DescriptorDeviation)]: { value: 42 } } } would have an value-mode path like ["foo", Symbol(DescriptorDeviation), "value"] which could not be distinguished from a similar descriptor-mode path for { foo: 42 }.

Thoughts on mismatchunequal, equality, or maybe even the equality algo used like same-zero? mismatch seems a little "shit happened" to me 😅

Yeah, I don't love "mismatch" either. "unequal" works for me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symbols are unique though, so Symbol(foo) ≠ Symbol(foo). If the symbol we use is well-known, it can be checked and there is no collision.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't stop someone from extracting that symbol and using it as a property key.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh. That's true… but then they clearly effed up, no? They broke themselves.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, there does not exist any value in JS that can be a "sentinel" value, because every such value could somehow appear in a user's object.

This claim is true but irrelevant here; we're talking about an array of property keys so objects are in fact available as sentinels.

Ahh. That's true… but then they clearly effed up, no? They broke themselves.

No, that's not clear at all. I am not willing to write off such use as "broken".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there is no legitimate use-case for a user to pass an object containing our own symbol to compare.

@gibson042 gibson042 Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's one: verification that an actual Symbol built-in matches expectations. There are multiple ways to do accomplish this, but at least one of them involves using the object itself (e.g., compare(globalThis.Symbol, expectedSymbol)).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Legitimate" isn't really relevant (although I can come up with some) - if it is possible, we have to define behavior for it, and having a special sentinel value produce confusing results just isn't viable.

A library can arbitrarily declare a subset of value types are valid, but the language can not.

@JakobJingleheimer

Copy link
Copy Markdown
Member Author

I think it's easy enough to get boolean output

This isn't about getting a boolean; it's purely about performance (and maybe a little bit about DX—but not related to boolean).

The only advantage of passing in a predicate rather than applying it to iterator output is that it allows filtering to affect boolean output. But it comes at the cost of making that mode intertwine implementation code with user code, which almost always incurs a performance penalty.

Before I opened this, I checked with Oli, who said this would likely have relatively significant perf benefits:

  • For each filtered deviation, it would save the cost of constructing the more expensive Deviation.
  • When all deviations are filtered out, it saves the cost of constructing the Iterable / Iterator.

@gibson042

Copy link
Copy Markdown
Member

The only advantage of passing in a predicate rather than applying it to iterator output is that it allows filtering to affect boolean output. But it comes at the cost of making that mode intertwine implementation code with user code, which almost always incurs a performance penalty.

Before I opened this, I checked with Oli, who said this would likely have relatively significant perf benefits:

  • For each filtered deviation, it would save the cost of constructing the more expensive Deviation.

I'm extremely skeptical here. Is he saying that constructing an { actual, expected, path, reason } object is more expensive than calling a user-defined function with (actual, expected, path, reason) arguments??

  • When all deviations are filtered out, it saves the cost of constructing the Iterable / Iterator.

Given how commonplace for...of loops are, I also don't buy this as a motivation.

Trying to optimize performance at this stage just seems misguided. From my perspective, the only argument to be made for filterer that makes sense is ergonomics of the boolean-output use case—e.g., const isUnchanged = !compare(old, new, { filterer }); rather than const isUnchanged = !compare(old, new, { mode: "full" })?.filter(filterer).next().value;.

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