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

[flow] annotate accumulate, accumulateInto and forEachAccumulated #7053

Merged
merged 1 commit into from Jun 17, 2016

Conversation

vjeux
Copy link
Contributor

@vjeux vjeux commented Jun 16, 2016

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

@vjeux vjeux changed the title [flow] annotate accumulate and accumulateInto [flow] annotate accumulate, accumulateInto and forEachAccumulated Jun 16, 2016
@ghost
Copy link

ghost commented Jun 16, 2016

@vjeux updated the pull request.

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
@ghost
Copy link

ghost commented Jun 16, 2016

@vjeux updated the pull request.

@zpao
Copy link
Member

zpao commented Jun 16, 2016

FYI #1 grunt flow should work too, but thanks for adding that to scripts.
FYI #2 npm run * saves you a bunch of typing over your lifetime. Also, npm test is a built in shortcut to the test script. (npm start is also a shortcut to the start script & falls back to node server.js. There are a couple others - restart is a fun one)

@keyz
Copy link
Contributor

keyz commented Jun 16, 2016

@vjeux Can we add a tracker about Flow-annotating the code base? I'd love to take some files in my spare time as well. We did a hackathon to annotate Jest's code base on Tuesday and it was pretty awesome.

@vjeux vjeux merged commit 5b4dad3 into facebook:master Jun 17, 2016
@vjeux
Copy link
Contributor Author

vjeux commented Jun 17, 2016

@keyanzhang are there any tracker tools already available? I'd love to see progress :)

@vjeux vjeux added this to the 15.2.0 milestone Jun 19, 2016
@zpao zpao modified the milestones: 15.2.0, 15-next Jun 22, 2016
zpao pushed a commit that referenced this pull request Jul 8, 2016
[flow] annotate accumulate, accumulateInto and forEachAccumulated
(cherry picked from commit 5b4dad3)
@zpao zpao modified the milestones: 15-next, 15.2.1 Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants