Type ReactComponentTreeHook #7504
Conversation
ghost
added
| }; | ||
|
|
||
| export type ReactElement = { | ||
| $$typeof: any, |
There was a problem hiding this comment.
I typed everything any that I did not need to type ReactComponentTreeHook
|
Sorry, I was still changing stuff and this got outdated. |
vjeux
force-pushed
the
type_ReactComponentTreeHook
branch
from
1290d53 to
ea4aecd
Compare
|
Just rebased |
For this one, I wanted to type a non-trivial piece of the codebase and ran into the fact that we do not have types for ReactElement nor ReactInstance, so I had to create them. I'll add comments inline
vjeux
force-pushed
the
type_ReactComponentTreeHook
branch
from
ea4aecd to
6842e2e
Compare
| } | ||
| function getIDFromKey(key: string): DebugID { | ||
| return parseInt(key.substr(1), 10); | ||
| } |
There was a problem hiding this comment.
lint is complaining about these functions being inside the else
116:3 error Move function declaration to program root no-inner-declarations
119:3 error Move function declaration to program root no-inner-declarations
There was a problem hiding this comment.
arg, I ran npm test but didn't run the lint. Travis is soo backed up, I'll send a quick fix
In Type ReactComponentTreeHook facebook#7504, I merged even though travis didn't report green (travis for all the fb repos has been backlogged like crazy since this morning) by manually doing `npm test` and `npm run flow` but I didn't ensure that lint was all green. @millermedeiros pinged me about it so here's a quick fix
| updateCount: number, | ||
| }; | ||
|
|
||
| declare function getItem(id: DebugID): ?Item; |
There was a problem hiding this comment.
Is this going to result in a global definition of getItem or is it local to this module? I thought these weren't supposed to be used in local code.
There was a problem hiding this comment.
I'm doing two implementations in the two branches below, I want both to match the same API. What is the flow way of doing things?
There was a problem hiding this comment.
Looks like I don't even need this block anymore, if I remove it, it works with flow!
There was a problem hiding this comment.
If we want to start using let/const consistently instead of var. We would have to hoist the let statements upwards.
Can you do something like this?
let foo : (x : number) => string;
if (window.bar) {
foo = (x : number) => x + 'bar';
} else {
foo = (x : number) => x + '';
}
let bar : string = foo(10);Flow is smart enough to know that it is not possible for foo to be undefined here.
There was a problem hiding this comment.
@sebmarkbage See #7570
In Type ReactComponentTreeHook #7504, I merged even though travis didn't report green (travis for all the fb repos has been backlogged like crazy since this morning) by manually doing `npm test` and `npm run flow` but I didn't ensure that lint was all green. @millermedeiros pinged me about it so here's a quick fix
For this one, I wanted to type a non-trivial piece of the codebase and ran into the fact that we do not have types for ReactElement nor ReactInstance, so I had to create them. I'll add comments inline (cherry picked from commit ea494a2)
In Type ReactComponentTreeHook #7504, I merged even though travis didn't report green (travis for all the fb repos has been backlogged like crazy since this morning) by manually doing `npm test` and `npm run flow` but I didn't ensure that lint was all green. @millermedeiros pinged me about it so here's a quick fix (cherry picked from commit 66e77f6)
For this one, I wanted to type a non-trivial piece of the codebase and ran into the fact that we do not have types for ReactElement nor ReactInstance, so I had to create them.
I'll add comments inline