feature(api): tell consumers when their objects are observable

[spiffytech]

See issue #483.

[coveralls]

Changes Unknown when pulling 081d723 on spiffytech:master into * on mobxjs:master*.

[mweststrate]

Thanks for the PR, will consider this for the next release

[mweststrate]

This typing is actually not correct, an object being converted to an observable is not an IObservable itself, it has an IObservable backing object instead, as defined here:

mobx/src/types/observableobject.ts

Line 51 in aa8ea15

export interface IIsObservableObject {

. I don't feel that exposing that interface however adds value, it is just the internal administration.

What could be done is introducing an empty IObservableObject interface. That has in itself not that much value, except that it could be used as typeguard for isObservableObject, so that people can express that they want to have some observable object as arguments to their functions. see also: #487

[spiffytech]

Having observable({}) return T & IObservableObject, with IObservableObject being an empty interface would satisfy my request.

Just to be clear, I'm not looking for TypeScript's type guards - they're designed to enhance type safety of code branches used in runtime type checks. I'm looking to improve compile-time checks, like so:

@action
function updateMyFoo(foo: MyInterface & IObservableObject) {
  foo.some_prop = true;  // triggers observers
}

const foo: MyInterface = {some_prop: false};
updateMyFoo(foo);  // compiler error - not observable

const observable_foo = observable(foo);  // MyInterfyace & IObservableObject
updateMyFoo(observable_foo);  // success

Would you like me to update my pull request to back out the use of IObservable, and create + use the empty IObservableObject interface?

I could add the type guard to isObservableObject a la #487 while I'm at it.

[mweststrate]

Yep, that makes sense to me :)

[Strate]

@spiffytech unfirtunatelly, due to structural typing system limitation (which TYpeScript is based on), you will not receive compile time errors here, because TS will not check interfaces on object, it will check structure of object. It means, that having empty interface is almost useless.
There is a suggestion to improve TS in this part, but still needs proposal: microsoft/TypeScript#202

[mweststrate]

dôh indeed

[Strate]

There is a cool trick, I've found in comments: microsoft/TypeScript#202 (comment)
Maybe we could use it here :)

[spiffytech]

@Strate Can you clarify how your suggestion works with empty interfaces? The below code sample compiles for me, when it should produce an error - updateMyFoo() is still allowing an argument of type MyInterface.

interface IObservableObject {}

interface MyInterface {
    some_prop: boolean;
}

function updateMyFoo(foo: MyInterface & IObservableObject) {
    foo.some_prop = true;
}

const foo: MyInterface = {some_prop: false};
const observable_foo = <MyInterface & IObservableObject> foo;

updateMyFoo(foo);  // compiles :(

updateMyFoo(observable_foo);  // compiles

[spiffytech]

@mweststrate May I assume from your previous comment that you wouldn't be comfortable exposing the IIsObservableObject interface (on an identical interface named IObservableObject), even if it's the only way to satisfy this feature request?

[mweststrate]

@spiffytech indeed, the mobx$ is an internal concept and should be hidden to the outside world as much as possible.

something like interface IObservableObject { "observable-object-marker": IObservableObject } is the trick I think. That attribute doesn't have to be exists at runtime, it is just that the compiler needs to think that it is returned from the observable function :)

[spiffytech]

Ah, I see! Confirmed, that makes the appropriate error occur in my code sample.

I'll update my full request with this.

[mweststrate]

@spiffytech did you by any chance proceed on this one ?

[spiffytech]

Yeah, I adopted what we discussed, added the type guard to isObservable, and added some unit tests for both.

I was hoping to find a way to unit test the failure case for the type guard, but everything I could think of prevented the TypeScript tests from compiling.

[mweststrate]

Just merged, thanks a lot!