ghcjs / ghcjs-base

base library for GHCJS for JavaScript interaction and marshalling, used by higher level libraries like JSC
MIT License
45 stars 67 forks source link

h$isObject is implemented incorrectly in `jsbits/utils.js`. #68

Open crocket opened 7 years ago

crocket commented 7 years ago

Let's have a look at https://github.com/ghcjs/ghcjs-base/blob/9d89e122eeeaeebfdb07a021cffb635532b1cbb7/jsbits/utils.js#L26-L28

function h$isObject(o) {
    return typeof(o) === 'object';
}

typeof(null) is 'object', thus you should replace that with something like

function h$isObject(o) {
    return (o instanceof Object);
}

I don't know if instanceof Object is totally accurate yet. I'll add comments on this.

crocket commented 7 years ago

According to http://stackoverflow.com/a/8511350/2104107 and http://stackoverflow.com/a/22482737/2104107,

function h$isObject(o) {
    return o !== null && typeof o === 'object';
}

seems correct. Since functions are considered objects in javascript specification, the following function could be considered correct, too.

function h$isObject(o) {
    return o !== null && (typeof o === 'object' || typeof o === 'function');
}
hamishmack commented 7 years ago

Is there an example use case where the current implementation is problematic? It seems like it would be best to mirror the behaviour of JavaScript in this case unless there is a reason not to.

crocket commented 7 years ago

I stopped programming for money. I'm transitioning into art. Nowadays, I rarely ever write code except when I write some haskell code in XMonad and small command line programs. Thus, I also stopped caring about user interface, javascript, and GHCJS.

That said, I don't know what you mean by mirroring the behavior of javascript. Does javascript have isObject? Anyway, I defer this issue entirely to you or anyone else.

hamishmack commented 7 years ago

I stopped programming for money.

Ah.

I'm transitioning into art.

Nice.

Nowadays, I rarely ever write code except when I write some haskell code in XMonad and small command line programs. Thus, I also stopped caring about user interface, javascript, and GHCJS.

Doh!

That said, I don't know what you mean by mirroring the behavior of javascript. Does javascript have isObject?

Good point. I guess I just was following the pattern that the other isX options were all the typeof of the value. We already have a wrapper for typeof so I think you are right it would be nicer for isObject to be a bit more OO in there behaviour (exclude null and include functions).

Also reading through that post in more detail suggests o === Object(o) works the same as (typeof(o) === 'object' \|\| typeof(o) === 'function') && o !== null. I think that might be the way to go because it could even be inlined inexpensively.