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

Backwards compatibility for string refs on WWW #28826

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,55 @@ describe('ReactComponent', () => {
}
});

// @gate !disableStringRefs
it('string refs do not detach and reattach on every render', async () => {
spyOnDev(console, 'error').mockImplementation(() => {});

let refVal;
class Child extends React.Component {
componentDidUpdate() {
// The parent ref should still be attached because it hasn't changed
// since the last render. If the ref had changed, then this would be
// undefined because refs are attached during the same phase (layout)
// as componentDidUpdate, in child -> parent order. So the new parent
// ref wouldn't have attached yet.
refVal = this.props.contextRef();
}

render() {
if (this.props.show) {
return <div>child</div>;
}
}
}

class Parent extends React.Component {
render() {
return (
<div id="test-root" ref="root">
<Child
contextRef={() => this.refs.root}
show={this.props.showChild}
/>
</div>
);
}
}

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(<Parent />);
});

expect(refVal).toBe(undefined);
await act(() => {
root.render(<Parent showChild={true} />);
});
expect(refVal).toBe(container.querySelector('#test-root'));
});

// @gate !disableStringRefs
it('should support string refs on owned components', async () => {
const innerObj = {};
Expand Down
19 changes: 19 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,25 @@ function markRef(current: Fiber | null, workInProgress: Fiber) {
);
}
if (current === null || current.ref !== ref) {
if (!disableStringRefs && current !== null) {
const oldRef = current.ref;
const newRef = ref;
if (
typeof oldRef === 'function' &&
typeof newRef === 'function' &&
typeof oldRef.__stringRef === 'string' &&
oldRef.__stringRef === newRef.__stringRef &&
oldRef.__stringRefType === newRef.__stringRefType &&
oldRef.__stringRefOwner === newRef.__stringRefOwner
) {
// Although this is a different callback, it represents the same
// string ref. To avoid breaking old Meta code that relies on string
// refs only being attached once, reuse the old ref. This will
// prevent us from detaching and reattaching the ref on each update.
workInProgress.ref = oldRef;
return;
}
}
// Schedule a Ref effect
workInProgress.flags |= Ref | RefStatic;
}
Expand Down
10 changes: 9 additions & 1 deletion packages/react/src/jsx/ReactJSXElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,15 @@ function coerceStringRef(mixedRef, owner, type) {
}
}

return stringRefAsCallbackRef.bind(null, stringRef, type, owner);
const callback = stringRefAsCallbackRef.bind(null, stringRef, type, owner);
// This is used to check whether two callback refs conceptually represent
// the same string ref, and can therefore be reused by the reconciler. Needed
// for backwards compatibility with old Meta code that relies on string refs
// not being reattached on every render.
callback.__stringRef = stringRef;
callback.__type = type;
callback.__owner = owner;
return callback;
}

function stringRefAsCallbackRef(stringRef, type, owner, value) {
Expand Down