ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 397 forks source link

(null == null) in the eyes of ractive.observer() even though typeof(null) is 'object' #307

Closed vsivsi closed 10 years ago

vsivsi commented 10 years ago

The documentation for ractive.observer() states: "no two objects are equal, even when they're identical", when referring to how equality is tested after a set in determining whether to notify an observer of a value change.

However, the current behavior (pre-v0.3.8) is that observer treats null == null, look at this fiddle for a demonstration.

typeof(null) == 'object', so there is an inconsistency here. I know that to Javascript, null === null because there is only one instance of null, but this is precisely in the same sense that the code below is true, which is explicitly addressed in the stated behavior of ractive.observer.

 a = {}; 
 b = a;
 a === b   // is true to Javascript, but false to ractive.observer()

I encountered this as the cause of a bug in real-world code, so it's not a purely academic issue. And it is icky to work around (using some other special sentinel value to stand in for null and then building-in checks everywhere for that special case... Blech.

Please consider fixing this for v0.3.8. Thanks.

vsivsi commented 10 years ago

This code appears to be the culprit. A special carveout for the equality of null. What is the reasoning behind this (in the context of observer)?

 isEqual = function ( a, b ) {
         if ( a === null && b === null ) {
                 return true;
         }

         if ( typeof a === 'object' || typeof b === 'object' ) {
                 return false;
         }

         return a === b;
 };

from: https://github.com/RactiveJS/Ractive/blob/d579a84d5d94903d20c292f6af28d901563d1471/src/internal/is.js#L8-L18

Rich-Harris commented 10 years ago

I've updated the documentation to clarify the situation.

I can't think of any situation where you'd want null values to be treated the same as other objects! The reason objects are normally treated as unequal whether or not they're 'similar' or 'identical' is that their properties might have changed, but null has no properties:

ractive.set( 'foo', null );
ractive.observe( 'foo', function ( newValue, oldValue ) {
  console.log( oldValue, 'changed to', newValue );
});

ractive.set( 'foo', null ); // 'foo' hasn't changed - the observer shouldn't fire!

So my inclination is to close this as wontfix - I don't think that null should be treated differently to undefined or true or false just because of a bug in the language (which is why that workaround exists). Can you clarify why it should?

vsivsi commented 10 years ago

Thanks for updating the docs, that helps. I don't have any desire to be pedantic about this, it's your system so you get to make the rules, and from what I've seen Ractive is well designed.

Javascript is what it is, bugs and all, and those of us who know it reasonably well read something like "no two objects are equal, even when they're identical" literally, because we know typeof(null) is 'object'. A case of knowing too much, perhaps. Here's how it got me into trouble:

Having understood the description above of "object inequality", I created a set of observers to keep a group of UI elements in a consistent state in the face of changes (both external, like user input or remote data changing, and internal, like other UI elements changing, with cascading effects on others...). These UI elements are represented as objects within my Ractive data, and I incorrectly assumed that if I gave the null value the semantics of "the element represented by this object is now invalid, and needs to be rebuilt", that I would get notified of all events making an element invalid (since each set to null would generate a new event). In this way of thinking, I'm accepting that typeof(null) is 'object', and using null semantically to mean "invalid" as distinguished from {} (valid, but empty).

In this sense, within the context of the behavior of ractive.observer(), I was treating null to be the object equivalent of NaN. Which, of course, you can't do in Javascript itself, because null === null (whereas NaN !== NaN, for good reason!). This makes a certain amount of sense though, because an object can become invalid for multiple reasons, and just because it's null already (meaning it hasn't been fixed up yet from the last time it was invalidated) doesn't mean I don't want to hear about new changes that have also made it invalid for potentially different reasons!

The workaround is to create some kind of special "invalid object" like, oh I don't know: { "_invalid_": true } which is guaranteed to be "unequal" with itself, and then test for that obj._invalid_ === true. Which is what I said "Yuck" to earlier. Maybe it's the 'C' programmer in me, but when confronted with a list of pointers to structures, I wouldn't consider the nonexistent structures represented by NULL (invalid) pointers to be "equal" in any meaningful way. And so within the context of the special rules for object equality used by ractive.observer, there is a certain logic to null !== null. If only it were true.

Rich-Harris commented 10 years ago

Ah, I see. I have a certain sympathy for the view that one null isn't the same as another null, but I think the semantics of null in JavaScript (i.e. null === null) favour treating it as the same value for the purpose of observers (especially since the 'objects are always unequal' thing is really just a cop-out that avoids having to do lots of potentially expensive cloning/deep comparison).

An alternative would be to have something like this:

ractive.observe( 'foo', function ( newValue, oldValue ) {
  console.log( oldValue, 'changed to', newValue );
}, { force: true });

ractive.set( 'foo', 'bar' ); // 'undefined changed to bar'
ractive.set( 'foo', 'bar' ); // 'bar changed to bar'

I've resisted this so far because it would mean a slight performance penalty for everyone (at present, operations short-circuit at certain points if there are no changes to propagate, and that would have to change in case there were any force: true observers).

Two potential workarounds would be to use NaN or a sentinel value e.g. var INVALID = {} - I know you expressed a preference for avoiding sentinel values, but you could argue that null is already a sentinel value in this context.

vsivsi commented 10 years ago

Thanks for your response. I ended up working around this by defining/firing my own invalidation events, bypassing the whole observer mechanism. i hadn't even considered using NaN as a sentinel; it's kind of hacktastic, but would work. The thing I preferred about using null is that it is falsy, unlike {}or { _invalid_: true }or whatnot, which makes the code dealing with it cleaner.

As an aside, prior to this it has never occurred to me that NaN is the only falsy value in JS that is also not equal to itself; which gives it the magical property I was looking for above (an observer will always see .set('blah',NaN) as a change, and the value itself evaluates to false in a test), if only it wasn't so hacky to (ab)use it in this way. Ah Javascript...

Rich-Harris commented 10 years ago

Yep, that's JS - there's always a way, it's just that it might make you gag! Will close this issue.