Skip to content

Type ReactOwner#7587

Merged
vjeux merged 1 commit intofacebook:masterfrom
vjeux:type_ReactOwner
Aug 28, 2016
Merged

Type ReactOwner#7587
vjeux merged 1 commit intofacebook:masterfrom
vjeux:type_ReactOwner

Conversation

@vjeux
Copy link
Copy Markdown
Contributor

@vjeux vjeux commented Aug 27, 2016

Incrementally type ReactInstance by adding the types of attach/detachRef.

I moved isValidOwner as a function inside of the file since it's never used externally.

Incrementally type ReactInstance by adding the types of attach/detachRef.

I moved isValidOwner as a function inside of the file since it's never used externally.
* @return {boolean} True if `object` is a valid owner.
* @final
*/
function isValidOwner(object: any): bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would you want to use 'mixed' here rather than 'any'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only call site of this function gives a variable of type ReactInstance, so if we follow the type system, this function is always going to return true and the invariant is dead code.

The only reason this function exists is that the entire code base is not respecting the type system. So adding a type mixed or any isn't going to make any difference :)

If you insist, I can change it to mixed, I don't really mind

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't worry about it since it makes no difference in this case - thanks for explaining. :)

@vjeux vjeux added this to the 15-next milestone Aug 28, 2016
@vjeux vjeux merged commit fa9869b into facebook:master Aug 28, 2016
@zpao zpao modified the milestones: 15-next, 15.4.0 Oct 4, 2016
zpao pushed a commit that referenced this pull request Oct 4, 2016
Incrementally type ReactInstance by adding the types of attach/detachRef.

I moved isValidOwner as a function inside of the file since it's never used externally.
(cherry picked from commit fa9869b)
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