Skip to content

Commit

Permalink
[flow] annotate accumulate and accumulateInto
Browse files Browse the repository at this point in the history
Summary:
Trying to start adding flow types to files in React. I needed to add a script to package.json in order to run flow, flow-bin is already a dependency.

I had to rewrite the implementation a bit. Flow doesn't recognize

```
var currentIsArray = Array.isArray(current);
if (currentIsArray) {
  // not refined
}
```

but the following does work

```
if (Array.isArray(current)) {
  // refined
}
```

Test Plan:
npm run-script flow
npm run-script test accumulate

Reviewers: @zpao @spicyj
  • Loading branch information
vjeux committed Jun 16, 2016
1 parent 49238b9 commit 1c71970
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 30 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@
"linc": "git diff --name-only --diff-filter=ACMRTUB `git merge-base HEAD master` | grep '\\.js$' | xargs eslint --",
"lint": "grunt lint",
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json",
"test": "jest"
"test": "jest",
"flow": "flow"
},
"jest": {
"modulePathIgnorePatterns": [
Expand Down
30 changes: 15 additions & 15 deletions src/shared/utils/accumulate.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule accumulate
* @flow
*/

'use strict';
Expand All @@ -20,28 +21,27 @@ var invariant = require('invariant');
*
* @return {*|array<*>} An accumulation of items.
*/
function accumulate(current, next) {
function accumulate<T>(current: ?(T | Array<T>), next: T | Array<T>): T | Array<T> {
invariant(
next != null,
'accumulate(...): Accumulated items must be not be null or undefined.'
);

if (current == null) {
return next;
} else {
// Both are not empty. Warning: Never call x.concat(y) when you are not
// certain that x is an Array (x could be a string with concat method).
var currentIsArray = Array.isArray(current);
var nextIsArray = Array.isArray(next);
if (currentIsArray) {
return current.concat(next);
} else {
if (nextIsArray) {
return [current].concat(next);
} else {
return [current, next];
}
}
}

// Both are not empty. Warning: Never call x.concat(y) when you are not
// certain that x is an Array (x could be a string with concat method).
if (Array.isArray(current)) {
return current.concat(next);
}

if (Array.isArray(next)) {
return [current].concat(next);
}

return [current, next];
}

module.exports = accumulate;
21 changes: 9 additions & 12 deletions src/shared/utils/accumulateInto.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule accumulateInto
* @flow
*/

'use strict';

var invariant = require('invariant');

/**
*
* Accumulates items that must not be null or undefined into the first one. This
* is used to conserve memory by avoiding array allocations, and thus sacrifices
* API cleanness. Since `current` can be null before being passed in and not
Expand All @@ -27,31 +27,28 @@ var invariant = require('invariant');
* @return {*|array<*>} An accumulation of items.
*/

function accumulateInto(current, next) {
function accumulateInto<T>(current: ?(T | Array<T>), next: T | Array<T>): T | Array<T> {
invariant(
next != null,
'accumulateInto(...): Accumulated items must not be null or undefined.'
);

if (current == null) {
return next;
}

// Both are not empty. Warning: Never call x.concat(y) when you are not
// certain that x is an Array (x could be a string with concat method).
var currentIsArray = Array.isArray(current);
var nextIsArray = Array.isArray(next);

if (currentIsArray && nextIsArray) {
current.push.apply(current, next);
return current;
}

if (currentIsArray) {
if (Array.isArray(current)) {
if (Array.isArray(next)) {
current.push.apply(current, next);
return current;
}
current.push(next);
return current;
}

if (nextIsArray) {
if (Array.isArray(next)) {
// A bit too dangerous to mutate `next`.
return [current].concat(next);
}
Expand Down
9 changes: 7 additions & 2 deletions src/shared/utils/forEachAccumulated.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule forEachAccumulated
* @flow
*/

'use strict';
Expand All @@ -18,12 +19,16 @@
* handling the case when there is exactly one item (and we do not need to
* allocate an array).
*/
var forEachAccumulated = function(arr, cb, scope) {
function forEachAccumulated<T>(
arr: ?(T | Array<T>),
cb: ((elem: T) => void),
scope: ?any,
) {
if (Array.isArray(arr)) {
arr.forEach(cb, scope);
} else if (arr) {
cb.call(scope, arr);
}
};
}

module.exports = forEachAccumulated;

0 comments on commit 1c71970

Please sign in to comment.