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] add some typings to utils #7104

Merged
merged 3 commits into from Jun 23, 2016
Merged

Conversation

chicoxyzzy
Copy link
Contributor

No description provided.

@chicoxyzzy chicoxyzzy changed the title add some typings to utils [flow] add some typings to utils Jun 23, 2016
@chicoxyzzy
Copy link
Contributor Author

@vjeux you are also working on typing so I decided to ask: which of rest utils files I can take?

@@ -20,7 +21,7 @@ var ReactNodeTypes = {
COMPOSITE: 1,
EMPTY: 2,

getType: function(node) {
getType: function(node: ReactElement): number {
Copy link
Contributor

@vjeux vjeux Jun 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReactElement<any>

Copy link
Contributor

@vjeux vjeux Jun 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning number, could you do

type ReactNodeType = 0 | 1 | 2;
getType: function(node: ReactElement<any>): ReactNodeType {

this way it'll prevent people from passing arbitrary numbers, flow will force to use the literal 0, 1, 2 or anything that returns those values. It's not perfect but much better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed

@vjeux
Copy link
Contributor

vjeux commented Jun 23, 2016

Thanks! I'm not working on anything else right now so feel free to take whichever you like :)

@@ -46,7 +52,7 @@ function flattenSingleChildIntoContext(traverseContext, child, name, selfDebugID
* children will not be included in the resulting object.
* @return {!object} flattened children keyed by name.
*/
function flattenChildren(children, selfDebugID) {
function flattenChildren(children: ReactElement<any>, selfDebugID: number): any {
Copy link
Contributor

@vjeux vjeux Jun 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you type the return function, if we put any, it doesn't bring any benefit since flow is just going to ignore it.

?{ [name: string]: ReactElement<any> }

@vjeux vjeux added this to the 15-next milestone Jun 23, 2016
@vjeux vjeux merged commit 3b5c756 into facebook:master Jun 23, 2016
@chicoxyzzy chicoxyzzy deleted the flow_utils branch June 23, 2016 16:21
traverseContext: mixed,
child: ReactElement<any>,
name: string,
selfDebugID: number
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just saw this. selfDebugID is actually an optional parameter that will only be available in dev mode. You can also see from the number= JSDoc annotation. cc @vjeux

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! #7110

zpao pushed a commit that referenced this pull request Jul 8, 2016
* add some typings to utils

* add typing of flattenChildren

* more accurate typings for flattenChildren

(cherry picked from commit 3b5c756)
@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

4 participants