-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
New Architecture Support - measureLayout() is obsolete with native node handlers #176
base: main
Are you sure you want to change the base?
New Architecture Support - measureLayout() is obsolete with native node handlers #176
Conversation
a2f51a6
to
3bc7a08
Compare
3bc7a08
to
7bdf1c8
Compare
@LunatiqueCoder I think for now I am going to be very dependent on your word for if/when this is ready to go. |
@lafiosca From my side, it's working like a charm. If I turn on the new architecture, there is no error. If I revert the changes, and I still have the new architecture on, I got the error:
Error is pretty self describing, and the fix is also easy 🚀 just deleted code and passed in the ref instead of the node handle :) |
@LunatiqueCoder One more request... can you identify which minimum version of RN is required for this and update the README and peer deps? |
@lafiosca I think this change shouldn't break older React Native versions. However, it would be nice to bump Also, I would suggest to use Expo's eslint config for OSS projects, it's pretty cool. |
@lafiosca The new architecture is now enabled by default on new react native projects. Soon will be for expo too. Let me know if I can help. I'm always available on Discord. |
Hi, I'm having an issue with the library with this patch but I haven't tested it with older (before new architecture) version yet. So can't say for sure if this PR is causing it but the issue is snapback of DraxViews after release. The animation does not work (it stops before finishing). E.g. if you run the KnightMoves example, after dragging the knight and releasing it, the dragged view will stay there and won't snap back. I've narrowed it down to this line:
Where the finished value will always be false, hence the view state won't get updated in the drag registry.
|
So, I checked with react native 0.75 with old architecture and there issue is still there so I think this might be a different issue. I'll open one for this. |
opened #180 |
|
@trantuandatvietnam Are you sure you added the patch correctly? |
I changed lib from react-native-drax to your pull request with |
@trantuandatvietnam I'm not sure that alone will work because it's written in TypeScript and the files have to be translated to JavaScript. You can check PR #177 , there are some clear instructions on how to test PR changes, along with Fast Refresh so you can switch branches and see changes reflecting immediately on the simulator. This way, you'll be able to reproduce it, and then test if the error goes away Add me on discord with the same user name (@LunatiqueCoder) if you need further help to test the changes. @lafiosca I resolved the comments, and retested. The error indeed goes away and The React Native docs have also been updated accordingly to better prevent this issue. 🚀 |
Here is the git diff to be used with
diff --git a/node_modules/react-native-drax/build/DraxList.js b/node_modules/react-native-drax/build/DraxList.js
index 456c190..e9ef709 100644
--- a/node_modules/react-native-drax/build/DraxList.js
+++ b/node_modules/react-native-drax/build/DraxList.js
@@ -45,10 +45,8 @@ const DraxListUnforwarded = (props, forwardedRef) => {
const reorderable = reorderableProp ?? (onItemReorder !== undefined);
// The unique identifer for this list's Drax view.
const id = (0, hooks_1.useDraxId)(idProp);
- // FlatList, used for scrolling.
+ // FlatList, used for scrolling and measuring children
const flatListRef = (0, react_1.useRef)(null);
- // FlatList node handle, used for measuring children.
- const nodeHandleRef = (0, react_1.useRef)(null);
// Container view measurements, for scrolling by percentage.
const containerMeasurementsRef = (0, react_1.useRef)(undefined);
// Content size, for scrolling by percentage.
@@ -177,7 +175,6 @@ const DraxListUnforwarded = (props, forwardedRef) => {
// Set FlatList and node handle refs.
const setFlatListRefs = (0, react_1.useCallback)((ref) => {
flatListRef.current = ref;
- nodeHandleRef.current = ref && (0, react_native_1.findNodeHandle)(ref);
if (forwardedRef) {
if (typeof forwardedRef === 'function') {
forwardedRef(ref);
@@ -491,7 +488,10 @@ const DraxListUnforwarded = (props, forwardedRef) => {
// Monitor drag drops to stop scrolling, update shifts, and possibly reorder.
const onMonitorDragDrop = (0, react_1.useCallback)((eventData) => handleInternalDragEnd(eventData, true), [handleInternalDragEnd]);
return (react_1.default.createElement(DraxView_1.DraxView, { id: id, style: style, scrollPositionRef: scrollPositionRef, onMeasure: onMeasureContainer, onMonitorDragStart: onMonitorDragStart, onMonitorDragOver: onMonitorDragOver, onMonitorDragExit: onMonitorDragExit, onMonitorDragEnd: onMonitorDragEnd, onMonitorDragDrop: onMonitorDragDrop },
- react_1.default.createElement(DraxSubprovider_1.DraxSubprovider, { parent: { id, nodeHandleRef } },
+ react_1.default.createElement(DraxSubprovider_1.DraxSubprovider, { parent: { id, viewRef: {
+ //@ts-ignore
+ current: flatListRef.current?.getNativeScrollRef()
+ } } },
react_1.default.createElement(react_native_1.FlatList, { ...flatListProps, style: flatListStyle, ref: setFlatListRefs, renderItem: renderItem, onScroll: onScroll, onContentSizeChange: onContentSizeChange, data: reorderedData }))));
};
exports.DraxList = (0, react_1.forwardRef)(DraxListUnforwarded);
diff --git a/node_modules/react-native-drax/build/DraxProvider.js b/node_modules/react-native-drax/build/DraxProvider.js
index 7b62fc2..843688f 100644
--- a/node_modules/react-native-drax/build/DraxProvider.js
+++ b/node_modules/react-native-drax/build/DraxProvider.js
@@ -34,7 +34,7 @@ const math_1 = require("./math");
const DraxProvider = ({ debug = false, style = styles.provider, children, }) => {
const { getViewState, getTrackingStatus, dispatch, } = (0, hooks_1.useDraxState)();
const { getAbsoluteViewData, getTrackingDragged, getTrackingReceiver, getTrackingMonitorIds, getTrackingMonitors, getDragPositionData, findMonitorsAndReceiver, getHoverItems, registerView, updateViewProtocol, updateViewMeasurements, resetReceiver, resetDrag, startDrag, updateDragPosition, updateReceiver, setMonitorIds, unregisterView, } = (0, hooks_1.useDraxRegistry)(dispatch);
- const rootNodeHandleRef = (0, react_1.useRef)(null);
+ const rootViewRef = (0, react_1.useRef)(null);
const handleGestureStateChange = (0, react_1.useCallback)((id, event) => {
if (debug) {
console.log(`handleGestureStateChange(${id}, ${JSON.stringify(event, null, 2)})`);
@@ -606,7 +606,7 @@ const DraxProvider = ({ debug = false, style = styles.provider, children, }) =>
updateViewMeasurements,
handleGestureStateChange,
handleGestureEvent,
- rootNodeHandleRef,
+ rootViewRef,
};
const hoverViews = [];
const trackingStatus = getTrackingStatus();
@@ -625,11 +625,8 @@ const DraxProvider = ({ debug = false, style = styles.provider, children, }) =>
}
}
});
- const setRootNodeHandleRef = (0, react_1.useCallback)((ref) => {
- rootNodeHandleRef.current = ref && (0, react_native_1.findNodeHandle)(ref);
- }, []);
return (react_1.default.createElement(DraxContext_1.DraxContext.Provider, { value: contextValue },
- react_1.default.createElement(react_native_1.View, { style: style, ref: setRootNodeHandleRef },
+ react_1.default.createElement(react_native_1.View, { style: style, ref: rootViewRef },
children,
react_1.default.createElement(react_native_1.View, { style: react_native_1.StyleSheet.absoluteFill, pointerEvents: "none" }, hoverViews))));
};
diff --git a/node_modules/react-native-drax/build/DraxScrollView.js b/node_modules/react-native-drax/build/DraxScrollView.js
index c7847ab..8e0e877 100644
--- a/node_modules/react-native-drax/build/DraxScrollView.js
+++ b/node_modules/react-native-drax/build/DraxScrollView.js
@@ -35,10 +35,8 @@ const DraxScrollViewUnforwarded = (props, forwardedRef) => {
const { children, style, onScroll: onScrollProp, onContentSizeChange: onContentSizeChangeProp, scrollEventThrottle = params_1.defaultScrollEventThrottle, autoScrollIntervalLength = params_1.defaultAutoScrollIntervalLength, autoScrollJumpRatio = params_1.defaultAutoScrollJumpRatio, autoScrollBackThreshold = params_1.defaultAutoScrollBackThreshold, autoScrollForwardThreshold = params_1.defaultAutoScrollForwardThreshold, id: idProp, ...scrollViewProps } = props;
// The unique identifer for this view.
const id = (0, hooks_1.useDraxId)(idProp);
- // Scrollable view, used for scrolling.
+ // Scrollable view, used for scrolling and measuring children
const scrollRef = (0, react_1.useRef)(null);
- // ScrollView node handle, used for measuring children.
- const nodeHandleRef = (0, react_1.useRef)(null);
// Container view measurements, for scrolling by percentage.
const containerMeasurementsRef = (0, react_1.useRef)(undefined);
// Content size, for scrolling by percentage.
@@ -168,7 +166,6 @@ const DraxScrollViewUnforwarded = (props, forwardedRef) => {
// Set the ScrollView and node handle refs.
const setScrollViewRefs = (0, react_1.useCallback)((ref) => {
scrollRef.current = ref;
- nodeHandleRef.current = ref && (0, react_native_1.findNodeHandle)(ref);
if (forwardedRef) {
if (typeof forwardedRef === 'function') {
forwardedRef(ref);
@@ -191,7 +188,7 @@ const DraxScrollViewUnforwarded = (props, forwardedRef) => {
return onScrollProp?.(event);
}, [onScrollProp]);
return id ? (react_1.default.createElement(DraxView_1.DraxView, { id: id, style: style, scrollPositionRef: scrollPositionRef, onMeasure: onMeasureContainer, onMonitorDragOver: onMonitorDragOver, onMonitorDragExit: resetScroll, onMonitorDragEnd: resetScroll, onMonitorDragDrop: resetScroll },
- react_1.default.createElement(DraxSubprovider_1.DraxSubprovider, { parent: { id, nodeHandleRef } },
+ react_1.default.createElement(DraxSubprovider_1.DraxSubprovider, { parent: { id, viewRef: scrollRef } },
react_1.default.createElement(react_native_1.ScrollView, { ...scrollViewProps, ref: setScrollViewRefs, onContentSizeChange: onContentSizeChange, onScroll: onScroll, scrollEventThrottle: scrollEventThrottle }, children)))) : null;
};
exports.DraxScrollView = (0, react_1.forwardRef)(DraxScrollViewUnforwarded);
diff --git a/node_modules/react-native-drax/build/DraxView.js b/node_modules/react-native-drax/build/DraxView.js
index 07bc812..8e18d65 100644
--- a/node_modules/react-native-drax/build/DraxView.js
+++ b/node_modules/react-native-drax/build/DraxView.js
@@ -62,19 +62,17 @@ const DraxView = ({ onDragStart, onDrag, onDragEnter, onDragOver, onDragExit, on
|| !!onMonitorDragDrop);
// The unique identifier for this view.
const id = (0, hooks_1.useDraxId)(idProp);
- // The underlying View, for measuring.
+ // The underlying View, for measuring and for subprovider nesting if this is a Drax parent view.
const viewRef = (0, react_1.useRef)(null);
- // The underlying View node handle, used for subprovider nesting if this is a Drax parent view.
- const nodeHandleRef = (0, react_1.useRef)(null);
// This view's measurements, for reference.
const measurementsRef = (0, react_1.useRef)(undefined);
// Connect with Drax.
- const { getViewState, getTrackingStatus, registerView, unregisterView, updateViewProtocol, updateViewMeasurements, handleGestureEvent, handleGestureStateChange, rootNodeHandleRef, parent: contextParent, } = (0, hooks_1.useDraxContext)();
+ const { getViewState, getTrackingStatus, registerView, unregisterView, updateViewProtocol, updateViewMeasurements, handleGestureEvent, handleGestureStateChange, rootViewRef, parent: contextParent, } = (0, hooks_1.useDraxContext)();
// Identify Drax parent view (if any) from context or prop override.
const parent = parentProp ?? contextParent;
const parentId = parent?.id;
// Identify parent node handle ref.
- const parentNodeHandleRef = parent ? parent.nodeHandleRef : rootNodeHandleRef;
+ const parentViewRef = parent ? parent.viewRef : rootViewRef;
// Register and unregister with Drax context when necessary.
(0, react_1.useEffect)(() => {
// Register with Drax context after we have an id.
@@ -263,13 +261,14 @@ const DraxView = ({ onDragStart, onDrag, onDragEnter, onDragOver, onDragExit, on
const measureWithHandler = (0, react_1.useCallback)((measurementHandler) => {
const view = viewRef.current;
if (view) {
- const nodeHandle = parentNodeHandleRef.current;
- if (nodeHandle) {
+ if (parentViewRef.current) {
const measureCallback = measurementHandler
? buildMeasureCallback(measurementHandler)
: updateMeasurements;
// console.log('definitely measuring in reference to something');
- view.measureLayout(nodeHandle, measureCallback, () => {
+ view.measureLayout(
+ // @ts-ignore
+ parentViewRef.current, measureCallback, () => {
// console.log('Failed to measure Drax view in relation to parent nodeHandle');
});
}
@@ -281,7 +280,7 @@ const DraxView = ({ onDragStart, onDrag, onDragEnter, onDragOver, onDragExit, on
// console.log('No view to measure');
}
}, [
- parentNodeHandleRef,
+ parentViewRef,
buildMeasureCallback,
updateMeasurements,
]);
@@ -395,7 +394,7 @@ const DraxView = ({ onDragStart, onDrag, onDragEnter, onDragOver, onDragExit, on
}
if (isParent) {
// This is a Drax parent, so wrap children in subprovider.
- content = (react_1.default.createElement(DraxSubprovider_1.DraxSubprovider, { parent: { id, nodeHandleRef } }, content));
+ content = (react_1.default.createElement(DraxSubprovider_1.DraxSubprovider, { parent: { id, viewRef } }, content));
}
return content;
}, [
@@ -404,11 +403,10 @@ const DraxView = ({ onDragStart, onDrag, onDragEnter, onDragOver, onDragExit, on
children,
isParent,
id,
- nodeHandleRef,
+ viewRef,
]);
const setViewRefs = (0, react_1.useCallback)((ref) => {
viewRef.current = ref;
- nodeHandleRef.current = ref && (0, react_native_1.findNodeHandle)(ref);
}, []);
return (react_1.default.createElement(react_native_gesture_handler_1.LongPressGestureHandler, { maxDist: Number.MAX_SAFE_INTEGER, shouldCancelWhenOutside: false, minDurationMs: longPressDelay, onHandlerStateChange: onHandlerStateChange, onGestureEvent: onGestureEvent /* Workaround incorrect typings. */, enabled: draggable },
react_1.default.createElement(react_native_1.Animated.View, { ...props, style: combinedStyle, ref: setViewRefs, onLayout: onLayout, collapsable: false }, renderedChildren)));
diff --git a/node_modules/react-native-drax/build/types.d.ts b/node_modules/react-native-drax/build/types.d.ts
index 9e8c720..e3c5027 100644
--- a/node_modules/react-native-drax/build/types.d.ts
+++ b/node_modules/react-native-drax/build/types.d.ts
@@ -1,5 +1,5 @@
import { RefObject, ReactNode } from 'react';
-import { ViewProps, Animated, FlatListProps, ViewStyle, StyleProp, ScrollViewProps, ListRenderItemInfo } from 'react-native';
+import { Animated, View, ViewProps, ViewStyle, StyleProp, FlatList, FlatListProps, ScrollView, ScrollViewProps, ListRenderItemInfo } from 'react-native';
import { LongPressGestureHandlerStateChangeEvent, LongPressGestureHandlerGestureEvent } from 'react-native-gesture-handler';
import { PayloadActionCreator, ActionType } from 'typesafe-actions';
/** Gesture state change event expected by Drax handler */
@@ -456,8 +456,8 @@ export interface DraxContextValue {
handleGestureStateChange: (id: string, event: DraxGestureStateChangeEvent) => void;
/** Handle gesture event for a registered Drax view */
handleGestureEvent: (id: string, event: DraxGestureEvent) => void;
- /** Root node handle ref for the Drax provider, for measuring non-parented views in relation to */
- rootNodeHandleRef: RefObject<number | null>;
+ /** Root View ref for the Drax provider, for measuring non-parented views in relation to */
+ rootViewRef: RefObject<View | null>;
/** Drax parent view for all views under this context, when nesting */
parent?: DraxParentView;
}
@@ -481,8 +481,8 @@ export interface DraxViewRegistration {
export interface DraxParentView {
/** Drax view id of the parent */
id: string;
- /** Ref to node handle of the parent, for measuring relative to */
- nodeHandleRef: RefObject<number | null>;
+ /** View Ref of the parent, for measuring relative to */
+ viewRef: RefObject<FlatList | ScrollView | View | null>;
}
/** Function that receives a Drax view measurement */
export interface DraxViewMeasurementHandler {
Thanks for the PR it solves the issue. |
|
I also face this problem. The reason is the ref for the FlatList is not a NativeComponent. Which can see in the devtools components panel. ![]() I don't konw the detail logic of this lib, just want to fix this issue. So the way to solve this is make the ref point at a real NativeComponent.
![]() Tip This fix has some trouble with scroll,please follow the new commit of LunatiqueCoder |
@zackdong1 Good catch. Checking. |
Fixed DraxList thanks to @zackdong1's 1st suggestion:
|
@LunatiqueCoder flatList?.scrollToOffset?.({ offset }); |
@trantuandatvietnam That's correct, I hit that error too. I just pushed again another fix with a combination of the fixes. Let me know how this goes for you @zackdong1 Thank you all for the help! :) really |
yeah,it's the correct fix. I only tested in my own project yesterday,which doesn't require scrolling。Apologize! |
Can someone please share information on how to test this patch fix? |
If this library exists on your project, you can search |
Just wanting to say that this branch solves the issues for both Android and iOS. Thanks! Also, there's no need to use "react-native-drax": "LunatiqueCoder/react-native-drax#new-architecture-support", Then if you execute either |
Here is my patch-package file: react-native-drax+0.10.3.patch I was able to build this patch file with the following steps:
Make a copy of the Revert the change in package.json. Install dependencies again. Replace the Once the files have been replaced, run:
Finally, generate the patch file:
|
Hello
This PR fixes:
The Problem
SOURCE: React Native Docs - Direct Manipulation: measureLayout()

Conclusion
On the new architecture,
measureLayout()
with arelativeToNativeNode
handler (instead of reference) is OBSOLETEI also opened a PR to update React Native Docs (which was already merged):
While passing a view ref will make it WORK with the new architecture, we should still use the new architecture capabilities.
From my testing this works fine, but I would love if the community reviews it and tests it.
Let me know if anyone needs help to test those changes!
Cheers 🍻