Backwards compatibility for string refs on WWW (#28826)
Seeing errors with undefined string ref values when trying to sync https://github.com/facebook/react/pull/28473 Added a test that reproduces the failing pattern. @acdlite pushed https://github.com/facebook/react/pull/28826/commits/a786481ae5702f1966ecdb62f3667f3d72966e78 with fix --------- Co-authored-by: Jack Pope <jackpope@meta.com> Co-authored-by: Andrew Clark <git@andrewclark.io>
Jack Pope committed
Apr 11, 2024 at 15:30 UTC
435415962371adc69e6a7211f7c65a5d72fc609c
3 files changed
+77
-1
packages/react-dom/src/__tests__/ReactComponent-test.js
+49
@@ -129,6 +129,55 @@ describe('ReactComponent', () => {
129
}
130
});
131
132
+ // @gate !disableStringRefs
133
+ it('string refs do not detach and reattach on every render', async () => {
134
+ spyOnDev(console, 'error').mockImplementation(() => {});
135
+
136
+ let refVal;
137
+ class Child extends React.Component {
138
+ componentDidUpdate() {
139
+ // The parent ref should still be attached because it hasn't changed
140
+ // since the last render. If the ref had changed, then this would be
141
+ // undefined because refs are attached during the same phase (layout)
142
+ // as componentDidUpdate, in child -> parent order. So the new parent
143
+ // ref wouldn't have attached yet.
144
+ refVal = this.props.contextRef();
145
+ }
146
+
147
+ render() {
148
+ if (this.props.show) {
149
+ return <div>child</div>;
150
+ }
151
+ }
152
+ }
153
+
154
+ class Parent extends React.Component {
155
+ render() {
156
+ return (
157
+ <div id="test-root" ref="root">
158
+ <Child
159
+ contextRef={() => this.refs.root}
160
+ show={this.props.showChild}
161
+ />
162
+ </div>
163
+ );
164
+ }
165
+ }
166
+
167
+ const container = document.createElement('div');
168
+ const root = ReactDOMClient.createRoot(container);
169
+
170
+ await act(() => {
171
+ root.render(<Parent />);
172
+ });
173
+
174
+ expect(refVal).toBe(undefined);
175
+ await act(() => {
176
+ root.render(<Parent showChild={true} />);
177
+ });
178
+ expect(refVal).toBe(container.querySelector('#test-root'));
179
+ });
180
+
181
// @gate !disableStringRefs
182
it('should support string refs on owned components', async () => {
183
const innerObj = {};
packages/react-reconciler/src/ReactFiberBeginWork.js
+19
@@ -1048,6 +1048,25 @@ function markRef(current: Fiber | null, workInProgress: Fiber) {
1048
);
1049
}
1050
if (current === null || current.ref !== ref) {
1051
+ if (!disableStringRefs && current !== null) {
1052
+ const oldRef = current.ref;
1053
+ const newRef = ref;
1054
+ if (
1055
+ typeof oldRef === 'function' &&
1056
+ typeof newRef === 'function' &&
1057
+ typeof oldRef.__stringRef === 'string' &&
1058
+ oldRef.__stringRef === newRef.__stringRef &&
1059
+ oldRef.__stringRefType === newRef.__stringRefType &&
1060
+ oldRef.__stringRefOwner === newRef.__stringRefOwner
1061
+ ) {
1062
+ // Although this is a different callback, it represents the same
1063
+ // string ref. To avoid breaking old Meta code that relies on string
1064
+ // refs only being attached once, reuse the old ref. This will
1065
+ // prevent us from detaching and reattaching the ref on each update.
1066
+ workInProgress.ref = oldRef;
1067
+ return;
1068
+ }
1069
+ }
1070
// Schedule a Ref effect
1071
workInProgress.flags |= Ref | RefStatic;
1072
}
packages/react/src/jsx/ReactJSXElement.js
+9
-1
@@ -1189,7 +1189,15 @@ function coerceStringRef(mixedRef, owner, type) {
1189
}
1190
}
1191
1192
- return stringRefAsCallbackRef.bind(null, stringRef, type, owner);
1192
+ const callback = stringRefAsCallbackRef.bind(null, stringRef, type, owner);
1193
+ // This is used to check whether two callback refs conceptually represent
1194
+ // the same string ref, and can therefore be reused by the reconciler. Needed
1195
+ // for backwards compatibility with old Meta code that relies on string refs
1196
+ // not being reattached on every render.
1197
+ callback.__stringRef = stringRef;
1198
+ callback.__type = type;
1199
+ callback.__owner = owner;
1200
+ return callback;
1201
}
1202
1203
function stringRefAsCallbackRef(stringRef, type, owner, value) {