paldepind / union-type

A small JavaScript library for defining and using union types.
MIT License
477 stars 28 forks source link

added support for instanceof with both the returned union Type and the sub type constructors #36

Open kwijibo opened 8 years ago

kwijibo commented 8 years ago

addresses problem mentioned in https://github.com/paldepind/union-type/issues/32

davidchambers commented 8 years ago

@paldepind, I urge you to reconsider supporting instanceof at all. It has caused many problems for both Ramda and Sanctuary. See plaid/sanctuary#100, for example.

kwijibo commented 8 years ago

@davidchambers to be clear, neither the existing master branch, nor this PR use instanceof (except in the tests admittedly). What this PR does is make the constructor create objects using the right prototype chain, so that given

const Animal = Type({Walrus: []})
const paul = Animal.Walrus()

both

R.is(Animal, paul)

and

R.is(Animal.Walrus, paul)

would return true.

kwijibo commented 8 years ago

@davidchambers Your description of instanceof in plaid/sanctuary#100 is interesting, but I don't find the behaviour of instanceof completely unreasonable. If you do an a instanceof b check, it's probably because you want to rely on a having a particular structure or behaviour that b guarantees. If a was actually just created by something with the same name/@@type as b, it might not behave in the required way.

davidchambers commented 8 years ago

What this PR does is make the constructor create objects using the right prototype chain, so that [R.is may be used on the values].

instanceof is fragile, as is any function which makes use of it (such as R.is). Consider this example:

$ cat index.js
const Foo = require('foo');
const isFoo = require('is-foo');

console.log(isFoo(new Foo()));

$ cat node_modules/foo/index.js
module.exports = function Foo() {};

$ cat node_modules/is-foo/index.js
const Foo = require('foo');

module.exports = x => x instanceof Foo;

$ cat node_modules/is-foo/node_modules/foo/index.js
module.exports = function Foo() {};

$ node index.js
false

Even though the two foo modules contain the same bytes, they are not identical in memory. This is the module-level equivalent of identity equality. In functional programming we should be operating at a higher level of abstraction: equivalence rather than identity.

kwijibo commented 8 years ago

@davidchambers it sounds like what you object to fundamentally, is javascript object equality? This has the same behaviour for the same reason I think (without using instanceof):

const isFoo = (function(){
 function Foo(){}
 return x => Foo.prototype.isPrototypeOf(x)
})()
function Foo(){}
const foo = new Foo()
isFoo(foo) //false

I don't find this surprising; it's consistent with javascript semantics (as I understand them). eg:

function isFoo(x){  var Foo = {}; return x==Foo }
var Foo = {}
isFoo(Foo) //false
paldepind commented 8 years ago

I agree with @davidchambers that instanceof is problematic. union-type currently uses isPrototypeOf which I think suffer from all the same problems.

davidchambers commented 8 years ago

I don't find this surprising; it's consistent with javascript semantics […].

Consistent, yes; helpful, no.

TypeError: ‘fromMaybe’ requires a value of type Maybe as its second argument; received Just(42)

Every module which exports functions which take or return values of the Maybe type must also export the type itself (even if it's defined by a third-party library).

const Maybe = require('maybe-type');
const quux = require('quux');

//    blah :: Maybe Number
const blah = quux.parseBlah('…');

//  Doesn't work, because `toArray` uses `instanceof` check.
Maybe.toArray(blah);

//  We must use `quux.Maybe.toArray` instead.
quux.Maybe.toArray(blah);

Furthermore, we may want to operate on multiple values of type Maybe a at once. What would we do if we had two such values created by "different" modules in memory? We might end up defining this:

//    foreignMaybeToMaybe :: ForeignMaybe a -> Maybe a
const foreignMaybeToMaybe = maybe =>
  maybe.isJust ? Maybe.Just(maybe.value) : Maybe.Nothing();

Let's avoid this complexity if possible. Let's provide a '@@type' property and avoid instanceof entirely.

jgoux commented 8 years ago

The @@type property would be a great solution! :+1: