Skip to content

Commit

Permalink
[fix] Order of ref merging
Browse files Browse the repository at this point in the history
Ensure internal refs are called before the forwardedRef. This ensures that the
DOM element mutation performed by usePlatformMethods occurs before user-space
refs are called.

Ref #1749
  • Loading branch information
necolas committed Oct 7, 2020
1 parent bdcb4de commit b432273
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
10 changes: 6 additions & 4 deletions packages/react-native-web/src/exports/Picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const Picker = forwardRef<PickerProps, *>((props, forwardedRef) => {
} = props;

const hostRef = useRef(null);
const setRef = useMergeRefs(forwardedRef, hostRef);

function handleChange(e: Object) {
const { selectedIndex, value } = e.target;
Expand All @@ -56,18 +55,21 @@ const Picker = forwardRef<PickerProps, *>((props, forwardedRef) => {
}
}

const supportedProps = {
const supportedProps: any = {
children,
disabled: enabled === false ? true : undefined,
onChange: handleChange,
ref: setRef,
style: [styles.initial, style],
testID,
value: selectedValue,
...other
};

usePlatformMethods(hostRef, supportedProps);
const platformMethodsRef = usePlatformMethods(supportedProps);

const setRef = useMergeRefs(hostRef, platformMethodsRef, forwardedRef);

supportedProps.ref = setRef;

return createElement('select', supportedProps);
});
Expand Down
7 changes: 4 additions & 3 deletions packages/react-native-web/src/exports/Text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ const Text = forwardRef<TextProps, *>((props, forwardedRef) => {

const hasTextAncestor = useContext(TextAncestorContext);
const hostRef = useRef(null);
const setRef = useMergeRefs(forwardedRef, hostRef);

const classList = [
classes.text,
Expand Down Expand Up @@ -155,10 +154,12 @@ const Text = forwardRef<TextProps, *>((props, forwardedRef) => {
supportedProps.dir = dir != null ? dir : 'auto';
}
supportedProps.onClick = handleClick;
supportedProps.ref = setRef;
supportedProps.style = style;

usePlatformMethods(hostRef, supportedProps);
const platformMethodsRef = usePlatformMethods(supportedProps);
const setRef = useMergeRefs(hostRef, platformMethodsRef, forwardedRef);

supportedProps.ref = setRef;

const element = createElement(component, supportedProps);

Expand Down
9 changes: 5 additions & 4 deletions packages/react-native-web/src/exports/TextInput/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,6 @@ const TextInput = forwardRef<TextInputProps, *>((props, forwardedRef) => {
[handleContentSizeChange]
);

const setRef = useMergeRefs(forwardedRef, hostRef, imperativeRef);

function handleBlur(e) {
TextInputState._currentlyFocusedNode = null;
if (onBlur) {
Expand Down Expand Up @@ -367,14 +365,17 @@ const TextInput = forwardRef<TextInputProps, *>((props, forwardedRef) => {
supportedProps.onKeyDown = handleKeyDown;
supportedProps.onSelect = handleSelectionChange;
supportedProps.readOnly = !editable;
supportedProps.ref = setRef;
supportedProps.rows = multiline ? numberOfLines : undefined;
supportedProps.spellCheck = spellCheck != null ? spellCheck : autoCorrect;
supportedProps.style = style;
supportedProps.type = multiline ? undefined : type;
supportedProps.inputMode = inputMode;

usePlatformMethods(hostRef, supportedProps);
const platformMethodsRef = usePlatformMethods(supportedProps);

const setRef = useMergeRefs(hostRef, platformMethodsRef, imperativeRef, forwardedRef);

supportedProps.ref = setRef;

return createElement(component, supportedProps);
});
Expand Down
7 changes: 4 additions & 3 deletions packages/react-native-web/src/exports/View/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ const View = forwardRef<ViewProps, *>((props, forwardedRef) => {

const hasTextAncestor = useContext(TextAncestorContext);
const hostRef = useRef(null);
const setRef = useMergeRefs(forwardedRef, hostRef);

const classList = [classes.view];
const style = StyleSheet.compose(
Expand Down Expand Up @@ -132,10 +131,12 @@ const View = forwardRef<ViewProps, *>((props, forwardedRef) => {

const supportedProps = pickProps(props);
supportedProps.classList = classList;
supportedProps.ref = setRef;
supportedProps.style = style;

usePlatformMethods(hostRef, supportedProps);
const platformMethodsRef = usePlatformMethods(supportedProps);
const setRef = useMergeRefs(hostRef, platformMethodsRef, forwardedRef);

supportedProps.ref = setRef;

return createElement('div', supportedProps);
});
Expand Down
14 changes: 5 additions & 9 deletions packages/react-native-web/src/hooks/usePlatformMethods.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@
* @flow
*/

import type { ElementRef } from 'react';

import UIManager from '../exports/UIManager';
import createDOMProps from '../modules/createDOMProps';
import { useImperativeHandle, useRef } from 'react';
import { useMemo, useRef } from 'react';

function setNativeProps(node, nativeProps, classList, pointerEvents, style, previousStyleRef) {
if (node != null && nativeProps) {
Expand Down Expand Up @@ -45,14 +43,12 @@ function setNativeProps(node, nativeProps, classList, pointerEvents, style, prev
* Adds non-standard methods to the hode element. This is temporarily until an
* API like `ReactNative.measure(hostRef, callback)` is added to React Native.
*/
export default function usePlatformMethods(hostRef: ElementRef<any>, props: Object) {
export default function usePlatformMethods(props: Object) {
const previousStyleRef = useRef(null);
const { classList, style, pointerEvents } = props;

useImperativeHandle(
hostRef,
() => {
const hostNode = hostRef.current;
return useMemo(
() => (hostNode: any) => {
if (hostNode != null) {
hostNode.measure = callback => UIManager.measure(hostNode, callback);
hostNode.measureLayout = (relativeToNode, success, failure) =>
Expand All @@ -63,6 +59,6 @@ export default function usePlatformMethods(hostRef: ElementRef<any>, props: Obje
}
return hostNode;
},
[hostRef, classList, pointerEvents, style]
[classList, pointerEvents, style]
);
}

0 comments on commit b432273

Please sign in to comment.