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.

Sign up for GitHub

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

Fix cloneElement using string ref w no owner #28797

Merged
merged 8 commits into from
Apr 9, 2024
Merged

Fix cloneElement using string ref w no owner #28797

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d2dd81
Fix cloneElement using string ref w no owner
josephsavona Apr 9, 2024
f7b42b3
Fix tests
rickhanlonii Apr 9, 2024
c97b811
add repro
josephsavona Apr 9, 2024
b7f5119
Merge branch 'fix-cloneelement-owner' of github.com:josephsavona/reac…
josephsavona Apr 9, 2024
28d13d2
fix test name
josephsavona Apr 9, 2024
bb30f2b
use @gate comment
josephsavona Apr 9, 2024
9aedf71
undo use of at-gate since it fails in other combinations
josephsavona Apr 9, 2024
5162111
maybe gate only on !disableStringRefs????
josephsavona Apr 9, 2024
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
8 changes: 6 additions & 2 deletions packages/react/src/__tests__/ReactElementClone-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,12 @@ describe('ReactElementClone', () => {
} else if (
gate(flags => flags.enableRefAsProp && !flags.disableStringRefs)
) {
expect(clone.ref).toBe(element.ref);
expect(clone.props).toEqual({foo: 'ef'});
expect(() => {
expect(clone.ref).toBe(element.ref);
}).toErrorDev('Accessing element.ref was removed in React 19', {
withoutStack: true,
});
expect(clone.props).toEqual({foo: 'ef', ref: element.ref});
Copy link
Member

@rickhanlonii rickhanlonii Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josephsavona since enableRefAsProp is on, the props will include the ref, so I updated the test.

} else {
// Not going to bother testing every possible combination.
}
Expand Down