-
Notifications
You must be signed in to change notification settings - Fork 47.8k
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
Add ref to Fragment (alternative) #32465
Open
jackpope
wants to merge
15
commits into
facebook:main
Choose a base branch
from
jackpope:fragment-refs-alt
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+1,132
−29
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eps1lon
reviewed
Feb 25, 2025
fb008d4
to
87d09a8
Compare
sebmarkbage
reviewed
Feb 25, 2025
sebmarkbage
requested changes
Feb 25, 2025
sebmarkbage
reviewed
Feb 26, 2025
3ae668f
to
0f40078
Compare
0f40078
to
2c926ba
Compare
hamlim
reviewed
Mar 5, 2025
Comment on lines
+57
to
+70
it('accepts a ref callback', async () => { | ||
let fragmentRef; | ||
const root = ReactDOMClient.createRoot(container); | ||
|
||
await act(() => { | ||
root.render( | ||
<Fragment ref={ref => (fragmentRef = ref)}> | ||
<div id="child">Hi</div> | ||
</Fragment>, | ||
); | ||
}); | ||
|
||
expect(fragmentRef.current).not.toEqual(null); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would fragmentRef
here have a .current
? Seems like it's not an instance of createRef
or useRef
so I wouldn't expect it to be "boxed" with a .current
but instead to be a direct reference to the reference?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This API is experimental and subject to change or removal.
This PR is an alternative to #32421 based on feedback: #32421 (review) . The difference here is that we traverse from the Fragment's fiber at operation time instead of keeping a set of children on the
FragmentInstance
. We still need to handle newly added or removed child nodes to apply event listeners and observers, so we treat those updates as effects.Fragment Refs
This PR extends React's Fragment component to accept a
ref
prop. The Fragment's ref will attach to a custom host instance, which will provide an Element-like API for working with the Fragment's host parent and host children.Here I've implemented
addEventListener
,removeEventListener
, andfocus
to get started but we'll be iterating on this by adding additional APIs in future PRs. This sets up the mechanism to attach refs and perform operations on children. The FragmentInstance is implemented inreact-dom
here but is planned for Fabric as well.The API works by targeting the first level of host children and proxying Element-like APIs to allow developers to manage groups of elements or elements that cannot be easily accessed such as from a third-party library or deep in a tree of Functional Component wrappers.
In this case, calling
fragmentRef.current.addEventListener()
would apply an event listener toA
,B
, andD
.C
is skipped because it is nested under the first level of Host Component. If another Host Component was appended as a sibling toA
,B
, orD
, the event listener would be applied to that element as well and any other APIs would also affect the newly added child.This is an implementation of the basic feature as a starting point for feedback and further iteration.