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

Iterable.reduce takes null as firstReduction if explicitly given. #1028

Closed
lukasbuenger opened this issue Dec 22, 2016 · 1 comment
Closed

Comments

@lukasbuenger
Copy link

lukasbuenger commented Dec 22, 2016

What happened

I wrote a super simple higher order function for List.reduce in order for reduce to be composable in a Ramda context:

const reduce = reducer => initialReduction => list => list.reduce(reducer, initialReduction); 

With this style, if I wanted to concat a list of strings, according to the docs I'd simply have to explicitly give the null as firstReduction:

const concatToString = reduce((acc, val) => acc.concat(val))(null);

From the docs:

If initialReduction is not provided, or is null, the first item in the Iterable will be used.

Turns out, that null (or undefined for what it's worth) is still passed as initialReduction which leads to unwanted behavior or even to errors.

Cause of error

https://github.com/facebook/immutable-js/blob/master/src/IterableImpl.js#L206

The check on L206 only takes the number of passed arguments into account, otherwise blindly assigns reduction = firstReduction (L209).

Possible solutions

a. Fix the docs
b. Fix the check

How to reproduce

Premise
const reduce = reducer => initialReduction => list => list.reduce(reducer, initialReduction); 
const addValues = reduce((a, b) => a + b)(null);
const concatValues = reduce((a, b) => a.concat(b))(null);

const listOfNumbers = Immutable.List.of(1, 2, 3)
const listOfStrings = Immutable.List.of('Hunter', 'S.', 'Thompson');
const listOfLists = Immutable.List.of(
  Immutable.List.of(1, 2),
  Immutable.List.of(3, 4),
  Immutable.List.of(5, 6)
);
Cases
// Expected 6
// This works as expected as the numeric value of `null` is 0.
addValues(listOfNumbers);
// 6
// Expected 'HunterS.Thompon' 
addValues(listOfStrings);
// 'nullHunterS.Thompon'
// Expected 'HunterS.Thompon' 
concatValues(listOfStrings);
// Uncaught TypeError: Cannot read property 'concat' of null
// Expected [1, 2, 3, 4, 5, 6] 
concatValues(listOfLists).toJS();
// Uncaught TypeError: Cannot read property 'concat' of null
@lukasbuenger lukasbuenger changed the title Iterable.reduce takes null as firstReduction if explicitly given. Iterable.reduce takes null as firstReduction if explicitly given. Dec 22, 2016
@leebyron
Copy link
Collaborator

leebyron commented Mar 3, 2017

Thanks for the report. The docs are out of date with the implementation which was updated to match the behavior of Array#reduce

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

2 participants