evilsoft / evilsoft/crocks

Apply algebra doesn't dispatch correctly to fantasy-land

Open
#440 6 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.