search

fantasyland / fantasy-land

Specification for interoperability of common algebraic structures in JavaScript
MIT License
10.12k stars 374 forks source link

Complete the implementations for primitive JS types #244

Closed i-am-tom closed 7 years ago

i-am-tom commented 7 years ago

Hello! I noticed the internal/patch.js file was looking a bit neglected, so I've added in all the sensible implementations I could think of for the primitive types. There are at least a couple we can't add because of limitations around type inference, but I left comments to point them out.

I'd also like to open the discussion as to whether Object can be considered a Profunctor type by the given implementation. I'm concerned that the keys in native JS objects aren't as flexible as one would hope, and you're obviously going to get funny behaviour with non-primitive types due to pointer lookups and other ugliness. I'm not optimistic, which is a shame :(

I've gone through every one and tested it in the console (and consequently fixed the chain implementation for Array, which was actually map). What do people think? :)

davidchambers commented 7 years ago

My hope is that once we merge fantasyland/fantasy-laws#1 we'll be able to remove the two files you've updated in this pull request (#174).

Thanks to sanctuary-type-classes it isn't necessary to mutate built-in prototypes in order to test conformance. You may like to submit a pull request to sanctuary-type-classes if it's missing some implementations, so your work will not be lost when we remove the assertion functions from this project.

i-am-tom commented 7 years ago

Oh :( So I wasted my time?

davidchambers commented 7 years ago

So I wasted my time?

I imagine it was a useful learning experience, if nothing else. :)

I believe you misunderstood the purpose of internal/patch.js, though. It was not intended to provide implementations for all the built-in types. Rather, it was a hack to get some tests to pass. We wanted to show that Identity a has a semigroup if a has a semigroup, so we modified String.prototype so we could concatenate Identity('abc') and Identity('def') to get Identity('abcdef').

i-am-tom commented 7 years ago

Unfortunately, these are all things I've implemented many times, hence my excitement to find a reusable place to put them, hah! Ah well. C'est la vie.

davidchambers commented 7 years ago

hence my excitement to find a reusable place to put them

Is sanctuary-type-classes not that place?

i-am-tom commented 7 years ago

Oh, I've no doubt! I'll PR anything (if there's anything) that's missing from sanctuary-type-classes over there when I next get some free time :D