Apply algebra doesn't dispatch correctly to fantasy-land
- Dominant language
- JavaScript
- Stars
- 1.6k
- Forks
- 103
- PR merge metrics
- No merged PRs in 30d
Description
Describing the bug in terms of the implementation:
- [`src/core/flNames.js`](https://github.com/evilsoft/crocks/blob/master/src/core/flNames.js) is missing the `ap` property.
- [`src/pointfree/ap.js`](https://github.com/evilsoft/crocks/blob/master/src/pointfree/ap.js#L20) is calling hardcoded `.ap`, instead of calling the fantasy-land method name, if available. See [map.js](https://github.com/evilsoft/crocks/blob/master/src/pointfree/map.js#L30) for inspiration.
**To Reproduce**
```js
const fl = require('fantasy-land')
const R = require('ramda')
const C = require('crocks')
const inspect = require('util').inspect.custom
class Just {
constructor(x) {
this.x = x
}
static of(x) {
return new Just(x)
}
[fl.map](f) {
return Just.of(f(this.x))
}
[fl.chain](f) {
return f(this.x)
}
[fl.ap](m) {
return m[fl.map](this.x)
}
[inspect]() {
return `Just ${this.x[inspect] ? this.x[inspect]() : this.x}`
}
}
console.log(R.map(R.add(5), Just.of(5)))
console.log(R.chain(x => Just.of(R.add(5, x)), Just.of(10)))
const add5 = Just.of(R.add(5))
// Manual `ap` call works:
console.log(add5[fl.ap](Just.of(15)))
// Ramda's `ap` works:
console.log(R.ap(Just.of(20))(add5))
// Crocks' `ap` FAILS:
console.log(C.ap(Just.of(25))(add5))
```
**Expected output**
```
Just 10
Just 15
Just 20
Just 25
Just 30
```
**Actual output**
```
Just 10
Just 15
Just 20
Just 25
TypeError: ap: Both arguments must be Applys of the same type
at ap (/node_modules/crocks/pointfree/ap.js:14:11)
at /node_modules/crocks/core/curry.js:25:12
at Object. (/test-ap-crocks.js:43:30)
```
**Possible solution**
I implemented a quick hack to fix it:
Change [`src/pointfree/ap.js:20`](https://github.com/evilsoft/crocks/blob/master/src/pointfree/ap.js#L20) to:
```js
return (m[fl.ap] || m.ap).call(x, m)
```
Add the `ap` property to [`src/core/flNames.js`](https://github.com/evilsoft/crocks/blob/master/src/core/flNames.js):
```js
ap: 'fantasy-land/ap'
```
**Actual solution**
Instead of maintaining a list of fantasy-land names in `flNames.js`, why not add the npm package fantasy-land as a dependency? It actually [exports this list of names](https://github.com/fantasyland/fantasy-land/blob/master/index.js). I bet more stuff is missing, or will be in the near future ;)
Contributor guide
Assessment
This issue has not been assessed yet.