Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is can be fooled #502

Closed
futpib opened this issue Mar 12, 2018 · 7 comments
Closed

is can be fooled #502

futpib opened this issue Mar 12, 2018 · 7 comments

Comments

@futpib
Copy link
Contributor

futpib commented Mar 12, 2018

S.is(function Number () {}, 4)
// -> true

While documentation says:

is :: TypeRep a -⁠> Any -⁠> Boolean

Takes a type representative and a value of any
type and returns true iff the given value is of the specified type.
Subtyping is not respected.

@davidchambers
Copy link
Member

S.is predates $.test. Now that the latter exists I suggest using it exclusively, as it's far more expressive:

> $.test ([]) ($.Number) (4)
true

> $.test ([]) ($.Array ($.Integer)) ([1, 2, 3])
true

> $.test ([]) ($.Array ($.Integer)) ([1, 2, 3.14])
false

Can we prevent S.is from being fooled without reintroducing the problems solved in #100?

Perhaps we should remove S.is. It was a reasonable stopgap, but we now have a good solution.

@gautaz
Copy link

gautaz commented Mar 16, 2018

Not sure about this but perhaps S.is could be kept as an alias for $.test(S.env)?

This would break the backward compatibility with the current S.is but as removing it seems an option, backward compatibility will be broken anyway.

@tomkel
Copy link

tomkel commented Mar 16, 2018

Yeah, it may be cleaner API-wise to only need to interact with properties on S.

David, are there any downsides to this alias?

@futpib
Copy link
Contributor Author

futpib commented Mar 16, 2018

only need to interact with properties on S.

You would need to export all type identifiers from $ to S to achieve that.

@gautaz
Copy link

gautaz commented Mar 16, 2018

You would need to export all type identifiers from $ to S to achieve that.

That's true, I was just advocating for a less radical approach such as writing things like S.is($.Integer) but I also understand the point raised by @tomkel.

A simple way to interact only with S would be something like S.is(S.$.Integer) but that seems ugly at first thought...

@davidchambers
Copy link
Member

Not sure about this but perhaps S.is could be kept as an alias for $.test(S.env)?

This is an interesting idea, @gautaz. It would make S.get, S.gets, and S.parseJson more convenient to use with Type values, diminishing the need for the variants proposed in #481.

@Avaq, what do you think of the possibility of repurposing S.is?

@Avaq
Copy link
Member

Avaq commented Mar 19, 2018

I prefer re-purposing S.is over #481.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants