Instances of global classes not observable

[scriptspry]

Test case:

https://runkit.com/scriptspry/observer-utils-global-definitions-not-observable

---

const {observable, observe, isObservable} = require('@nx-js/observer-util');

class Counter {
    constructor() { this._count = 0 }
    increment() { this._count += 1 }
    decrement() { this._count -= 1 }
}

const c = observable(new Counter());
console.log('c', isObservable(c));

global.Counter = Counter;
const c2 = observable(new Counter());
console.log('c2', isObservable(c2));

[scriptspry]

I understand this is due to shouldInstrument function check.. Would like to know the reasoning behind this check. Sorry if this was documented already somewhere.

[solkimicreb]

Hi!

That's intended, but it is a dirty hack and it should be changed. I introduced this in the latest patch (4.1.2). The reason:

Built-in JS objects can not be wrapped with Proxies because of their 'internal slots'. They throw an 'Invalid Invocation' error when you try to call their methods on the wrapping Proxy. To work around this I had to do two things.

1. Instrument stateful built-in objects - like Map and Set - with monkey patching instead of Proxies.
2. Opt-out stateless built-in objects from the Proxy wrapping, since there is not state to observe and and it would throw errors.

To do the latter I need an algorithm, which detects built-in Objects. At first I tried to maintain a list, since there or not so many standard JS built-ins. I failed to do this though, because this lib supports both NodeJS and browsers, which have a lot of different built-ins. I got issues about illegal invocation error on WebAudio objects (browser built-in) for example. Maintaining a list of all browser and NodeJS built-ins is not feasible, so I decided to do the current dirty detection. I check if the object constructor is available on the global Object, since all built-ins are global.

This should swapped with a more elegant algorithm, but I couldn't find any. I will keep thinking about this and change it soon and I am open to any suggestions, ideas and PRs.

I hope this made the issue clear. Thanks for the issues 🙂 (I won't be around during the weekend.)

[solkimicreb]

Hi,

I couldn't find any nice general way to detect built-ins yet. One improvement I could do is to save all global constructors in a Set at startup time and use it later, instead of dynamically checking the global object each time. It means it would not instrument only objects that has a constructor, which was global when the library initialized. This way your specific example would work.

Still if you define some custom global constructors before you import the lib, it won't instrument instances of that constructor. I am looking for a better alternative 🙁

[scriptspry]

This repo is 8 years old, but the getCleanWindow seems promising. If it isn't costly on performance and about:blank is robust enough url to load across browsers, this could help in checking if it's a browser native or a user defined property on the window.

[scriptspry]

On second thought, I don't think that will work, It will just check for things in Window but not things like Map, Array etc. which are what need to be skipped. I have no other ideas. Perhaps posting on stackoverflow may help us.

EDIT: posted too soon. I think this might still work. Object.keys(frame.contentWindow) didn't have Map, Array etc. but 'Array' in frame.contentWindow is true. So that might just work.