typeforce
typeforce copied to clipboard
Change quacksLike to check expected properties, not name
@weilu you mentioned this in https://github.com/dcousens/typeforce/commit/3c8499b0f393f5d06642aa2b1ebafab2b0c0c9bf, however to enforce this we would need to pass in the type value as it stands and then build an actual type structure from that object.
Aka
var x = { a: 1 }
var y = { b: 1 }
var tx = typeforce.quacksLike(x)
// returns equivalent of { a: "Number" }
typeforce(typeforce.quacksLike(x), y)
// TypeError: Expected property "a" of type Number, got undefined
Thoughts?
I can't see how to do this without a breaking change
This would definitely be useful. The current implementation breaks when you mangle constructor names, e.g. with Uglify.
What are you concerned with it breaking?
It is a breaking change insofar as it indiscriminately allows any type if uglify
is used.
In libraries that use this module, we recommend them to:
If you uglify the javascript, you must exclude the following variable names from being mangled: BigInteger, ECPair, Point. This is because of the function-name-duck-typing used in typeforce.
uglifyjs ... --mangle --reserved 'BigInteger,ECPair,Point'
Apologies, @treygriffith I thought I was replying to https://github.com/dcousens/typeforce/pull/47
The breaking change here is that Users are currently passing a String
, and we are changing the API to be an instantiated object with property matching.
@dcousens yeah I see that now. The way to introduce this might be through a new method (shame since the method name quacksLike
is so good!).
Thanks for the note on the ECPair
, Point
, and BigInteger
- I only found one of those myself, so will be good to preemptively add those others to the reserved
list.
@treygriffith major bump is how I would do it, tbh.
@dcousens If you're open to PRs, I can take a first pass at this.
@treygriffith I am open to it, I just can't guarantee when it will merge.
https://github.com/dcousens/typeforce/pull/45 is something I'd additionally want to solve by 2.0.0
... so open to your thoughts there too.
Ok, I'm thinking through this a bit more, and I want to get your opinion before I proceed.
It seems the main use of this function is to determine whether value
is constructed of Type
. The way this is handled now is by checking whether the constructor.name
of value
is equivalent to the string type
.
There are other ways to accomplish something similar, each with their own drawbacks:
1. Checking value instanceof Type
.
Pros:
- fixes the mangling issue since we're checking the actual constructor
Cons:
- if
Type
is imported twice, this check may fail (an edge case, but difficult to debug) - if
value
is constructed of an extension ofType
, this check will fail - if
value
exhibits the properties ofType
but is not literally constructed, this check will fail
2. Checking Type.prototype.isPrototypeof(value)
Pros:
- fixes the mangling issue
- if
value
is constructed of an extension ofType
this check will work - if
value
is constructed by copying methods/properties fromType.protoype
, but is not literally constructed, this check will work (e.g.Object.create
)
Cons:
- if
Type
is imported twice, this check may fail (an edge case, but difficult to debug) - if
value
exhibits the properties ofType
but is not created, this check will fail (e.g.Object.assign
)
3. Iterating over the methods/properties of Type.prototype
to ensure equality with value
Pros:
- fixes the mangling issue
- if
value
is constructed of an extension ofType
this check will work - if
value
has copied methods/properties fromType.protoype
, but is not literally constructed, this check will work (e.g.Object.create
,Object.assign
)
Cons:
- if
Type
is imported twice, this check may fail (an edge case, but difficult to debug) - if
value
has methods/child objects that do the same thing functionally asType
, but are not literally copied, this check will fail
4. Iterating over the methods/properties of Type.prototype
to ensure type equivalence with value
Pros:
- fixes the mangling issue
- if
value
is constructed of an extension ofType
this check will work - if
value
has copied methods/properties fromType.protoype
, but is not literally constructed, this check will work (e.g.Object.create
,Object.assign
) - if
Type
is imported twice, this check will work - if
value
has methods/child objects that are of the same type but not literally copied, this check will work
Cons:
- Introduces a "false positive" possibility where
value
can "look like"Type
by having functions with the same names, but not implementing those functions the same way.
My proposal would be to do 2 with a fallback to 4. What do you think @dcousens ? Is the false positive here acceptable, or are we better off rejecting value
s that are similar but not are not constructed of Object.create
d?
Additionally, is there a desire to extend this beyond constructors? If so, it may make sense to generalize this even further to have the quacksLike
signature look like quacksLike(Foo.prototype)
, instead of quacksLike(Foo)
(both in contrast to the current signature: quacksLike('Foo')
)
Can't respond right now, will tonight.
if Type is imported twice, this check may fail (an edge case, but difficult to debug)
This issue is why quacksLike
was introduced the way it is.
Introduces a "false positive" possibility where value can "look like" Type by having functions with the same names, but not implementing those functions the same way.
I think this is a great solution personally. It is by definition a great way to enforce quacksLike
, better than focusing on the data too.
We could make it a score based system. 90% alike etc...
I kind of figured that might be the case. I'll re-work the PR to do a type equivalence check on all the properties.