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: View can be a string? #15955

Closed
Ashoat opened this issue Sep 14, 2017 · 9 comments
Closed

Flow: View can be a string? #15955

Ashoat opened this issue Sep 14, 2017 · 9 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@Ashoat
Copy link
Contributor

Ashoat commented Sep 14, 2017

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

  1. react-native -v:
react-native-cli: 2.0.1
react-native: 0.48.2
  1. node -v:
v8.4.0
  1. npm -v:
5.4.1
  1. flow version:
Flow, a static type checker for JavaScript, version 0.51.1

Then, specify:

  • Target Platform: N/A, Flow static typesystem

  • Development Operating System: macOS

  • Build tools: N/A, Flow static typesystem

Steps to Reproduce

render() {
  return <View ref={(view: ?View) => {}} />;
}
  1. Flow errors out because it thinks View is a string.

Expected Behavior

View should be a React.Component.

Actual Behavior

  1. View is the result of calling requireNativeComponent()
  2. requireNativeComponent returns React$ComponentType<any> | string
  3. Consequently, Flow thinks View can be a string

Reproducible Demo

// @flow

import React from 'react';
import { View } from 'react-native';

class Test extends React.PureComponent {

  render() {
    return <View ref={(view: ?View) => {}} />;
  }

}
@Ashoat
Copy link
Contributor Author

Ashoat commented Sep 14, 2017

@bvaughn - any chance you can elucidate why View can be a string? You added a type suppression relating to this here. Looks like it has to do with something in React Fiber?

Can we not reliably depend on the ref returned from View being a React.Component? Or is the export from the react-native package typed incorrectly?

@bvaughn
Copy link
Contributor

bvaughn commented Sep 14, 2017

Can we not reliably depend on the ref returned from View being a React.Component?

Unfortunately, that is correct. Native view components ("host components") are represented as strings with React 16. This is true for React DOM (eg "div", "span", etc.) and React Native (eg "RCTView", "RCTText", etc). React.Component (or React.PureComponent) are classes you extend to create JavaScript components.

Previously, React 15 created a wrapper object around host components (which is why the Flow typing "worked" before) but this was inefficient so we removed it for 16.

Internally, it looks like people are setting the ref-type to a mix of any or NativeMethodsMixinType (but I don't think this latter type is accessible to OSS users).

@Ashoat
Copy link
Contributor Author

Ashoat commented Sep 15, 2017

Thanks for responding!!

Any idea if there is a way to get to View.measure() in this new world? Unfortunately this old API is the only current way to fetch the absolute position of a View on the screen (onLayout doesn't have this info: #10556). Fetching the absolute position might seem unnecessary, but unfortunately PanResponder's local position information is double broken (#12591, #15290), so absolute position information is necessary for the comparison.

Ideally the local position issues will be fixed, and if not hopefully the absolute positions will be added to the onLayout callback. But failing either of those things happening, View.measure() is currently the only lifeline for tracking the x, y coordinates of a pan relative to a View.

@Ashoat
Copy link
Contributor Author

Ashoat commented Sep 15, 2017

I had a new thought in bed last night: if people within Facebook are typing the parameter passed in to the ref callback prop as a NativeMethodsMixinType, that means it can't be a string, right?

It seems like the T in React.Element<T> - ie. the "tag name" used when writing JSX - can now be a string as well as a React.Component. But perhaps the parameter passed to the ref callback will continue being a React.Component, or at least something that still is a NativeMethodsMixinType.

If that's the case, I just need to type the parameter to the ref callback differently - probably mirroring NativeMethodsMixinType (which I think I can sneakily include using an exact path into react-native).

My main question is: can the parameter to the ref callback be a string too? Or is that only the View imported from the react-native package?

@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2017

That's correct @Ashoat, the value of ref is not a string. The value of View is a string though1. So when you render a native View you're really creating an element with type "RCTView" (similar to how rendering an HTMLDivElement in the DOM is accomplished by creating an element with type "div"). The string just signals to React fiber which element type to create, native or not.

For native components, React Native renderer creates an instance of ReactNativeFiberHostComponent which is what gets passed to the ref callback. This class defines methods like measure and setNativeProps that you can use to interact with the backing native view. Unfortunately this type isn't exposed via the React Native public API so you can't use it for the type.

For the time being, you could copy the following Flow types from the react codebase:

type MeasureOnSuccessCallback = (
  x: number,
  y: number,
  width: number,
  height: number,
  pageX: number,
  pageY: number,
) => void;

type MeasureInWindowOnSuccessCallback = (
  x: number,
  y: number,
  width: number,
  height: number,
) => void;

type MeasureLayoutOnSuccessCallback = (
  left: number,
  top: number,
  width: number,
  height: number,
) => void;

// This would be the "ref" type for host components
type NativeMethodsMixinType = {
  blur(): void,
  focus(): void,
  measure(callback: MeasureOnSuccessCallback): void,
  measureInWindow(callback: MeasureInWindowOnSuccessCallback): void,
  measureLayout(
    relativeToNativeNode: number,
    onSuccess: MeasureLayoutOnSuccessCallback,
    onFail: () => void,
  ): void,
  setNativeProps(nativeProps: Object): void,
};

cc @calebmer: Let's talk about a way to make this work better for open source React Native + Flow users? (Maybe a way exists already that I'm unfamiliar with?)

1 The value of View is a string in production but it has a React.Component based wrapper in DEV mode so that some additional validation can be done.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2017

Unfortunately I can't assign this issue to myself for some reason, but I'll create a PR shortly that will hopefully provide a meaningful default Flow type for host components. Will post back here once it's done.

@Ashoat
Copy link
Contributor Author

Ashoat commented Sep 15, 2017

My current solution is just:

import type {
  NativeMethodsMixinType,
} from 'react-native/Libraries/Renderer/shims/ReactNativeTypes';

And then use NativeMethodsMixinType where I previously was using View.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2017

Using NativeMethodsMixinType for your refs for now seems like the best solution, however you declare or import it. I'll try to land some .js.flow files for View and the other host component types so this will just work in a future release and won't require any funky typing like this.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2017

FYI this issue should be fixed by 11b4084. Someone (who has permissions to do so) can probably close this issue.

@Ashoat Ashoat closed this as completed Sep 16, 2017
@facebook facebook locked as resolved and limited conversation to collaborators Sep 16, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Sep 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

3 participants