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

[Fiber] Add types for ReactFiber and ReactChildFiber #8072

Merged
merged 2 commits into from
Oct 24, 2016

Conversation

koba04
Copy link
Contributor

@koba04 koba04 commented Oct 24, 2016

It might be unnecessary because Flow can infer their return types.
But I think the type annotations for ReactFiber in this is valuable because it's module boundaries.

@gaearon
Copy link
Collaborator

gaearon commented Oct 24, 2016

Let's get it in, @sebmarkbage can tweak types later if he needs to. 😉

@gaearon gaearon merged commit 6cdc610 into facebook:master Oct 24, 2016
@gaearon gaearon added this to the 15-next milestone Oct 24, 2016
@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Oct 24, 2016

This is great! On a meta-level though. Currently Fiber isn't properly typed. It has a bunch of any in there. I think that once the code settles a bit we will start creating more specific fiber types such as ClassComponentFiber<T> so that we're safely able to track specific types of fibers and avoid unsound downcasts or any. When that happens, it might be a lot more convenient to rely on the inference rather than adding complex type annotations at every callsite. However, we can add * annotations at that point if we need to. So I don't think we necessarily should make it best practice to always annotate the return value. Even though it may seem like a good practice, it can also create dogma that makes it harder to type things properly.

@koba04
Copy link
Contributor Author

koba04 commented Oct 25, 2016

I got it, that makes sense! Thanks!

@koba04 koba04 deleted the add-types-for-react-fiber branch October 25, 2016 04:40
@gaearon gaearon removed this from the 15-next milestone Jan 6, 2017
acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
* Add Types for ReactFiber

* Add Types for ReactChildFiber
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.

4 participants