Add observer methods to fragment instances #32619
Conversation
github-actions
bot
added
the
React Core Team
|
Comparing: df31952...765f9dc Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
| @@ -0,0 +1,96 @@ | |||
| import TestCase from '../../TestCase'; | |||
There was a problem hiding this comment.
This is just moving into a new module from index.js
| @@ -0,0 +1,77 @@ | |||
| /** | |||
There was a problem hiding this comment.
Moved these utilities out of ReactDOMTestSelectors-test.js so we can reuse in ReactDOMFragmentRefs-test.js
I forgot why I even mentioned this originally. To clarify, I believe it's because IntersectionObservers don't fire any more events if you unobserve them. So if we unobserved them, there wouldn't be any more events fired and you wouldn't know to track them. If a child isn't deleted but just goes Offscreen while the parent Fragment is still mounted, then presumably the same thing effectively happens since it will be invisible due to A similar thing is what should happen when the Fragment itself unmounts? The issue is that the empty rect events in that scenario fires, after it has unmounted. If those then call setState that suggests a leak since there are setStates happening on an already unmounted. We have currently relaxed the setState warnings due to Promises so often leading to temporary leaks by not implementing aborting but we could do a version of it still and it suggests that something isn't quite right. We could consider calling In other words, to use this correctly you should actually do this: Stashing the observer in a ref is a sign of an anti-pattern because you're only doing that because you're not cleaning it up. You should update the fixtures to use this pattern instead. |
| 'You are attaching an observer to a fragment instance that already has one. Fragment instances ' + | ||
| 'can only have one observer. Use multiple fragment instances or first call unobserveUsing() to ' + | ||
| 'remove the previous observer.', | ||
| ); |
There was a problem hiding this comment.
What's the reason for this limitation? It doesn't seem strictly necessary to be able to have multiple observers but also not necessary to limit it since it's just an array.
In the original RFC I mentioned this:
Since this causes the observer events to be passed the underlying DOM nodes as the target it can be hard to keep track of which Fragment ref they belong to so it can be best to keep on observer per Fragment.
To clarify that point, there can still be many-to-many. I meant that users should probably create one Observer for every Fragment that they want to observe but you can still many Observers observing a single Fragment. It doesn't have to be strictly enforced but just best practice.
There was a problem hiding this comment.
I was going back and forth on this and ended up thinking it might be simpler to start with a single observer per instance. But it doesn't really take away anything to make it more flexible. I'll extend it to handle multiple and drop this error.
|
The reason for storing the observer on a ref in IntersectionObserverCase is to access it from the "Observe"/"Unobserve" click handlers on the buttons within that component. Otherwise yeah it can just be created and cleaned up in the effect itself. I like the idea of making it the creators responsibility to clean up but providing a warning on unmount. I'll follow up with that change. |
jackpope
force-pushed
the
fragment-refs-observe
branch
from
40328d6 to
6e372d6
Compare
This implements `observeUsing(observer)` and `unobserverUsing(observer)`
on fragment instances. IntersectionObservers and ResizeObservers can be
passed to observe each host child of the fragment. This is the
equivalent to calling `observer.observe(child)` or
`observer.unobserve(child)` for each child target.
Just like the addEventListener, the observer is held on the fragment
instance and applied to any newly mounted child. So you can do things
like wrap a paginated list in a fragment and have each child
automatically observed as they commit in.
Unlike, the event listeners though, we don't `unobserve` when a child is
removed. If a removed child is currently intersecting, the observer
callback will be called when it is removed with an empty rect. This lets
you track all the currently intersecting elements by setting state from
the observer callback and either adding or removing them from your list
depending on the intersecting state. If you want to track the removal of
items offscreen, you'd have to maintain that state separately and append
intersecting data to it in the observer callback. This is what the
fixture example does.
There could be more convenient ways of managing the state of multiple
child intersections, but basic examples are able to be modeled with the
simple implementation. Let's see how the usage goes as we integrate this
with more advanced loggers and other features.
For now you can only attach one observer to an instance. This could
change based on usage but the fragments are composable and could be
stacked as one way to apply multiple observers to the same elements.
In practice, one pattern we expect to enable is more composable logging
such as
```javascript
function Feed({ items }) {
return (
<ImpressionLogger>
{items.map((item) => (
<FeedItem />
))}
</ImpressionLogger>
);
}
```
where `ImpressionLogger` would set up the IntersectionObserver using a
fragment ref with the required business logic and various components
could layer it wherever the logging is needed. Currently most callsites
use a hook form, which can require wiring up refs through the tree and
merging refs for multiple loggers.
DiffTrain build for [cd28a94](cd28a94)
This implements
observeUsing(observer)andunobserverUsing(observer)on fragment instances. IntersectionObservers and ResizeObservers can be passed to observe each host child of the fragment. This is the equivalent to callingobserver.observe(child)orobserver.unobserve(child)for each child target.Just like the addEventListener, the observer is held on the fragment instance and applied to any newly mounted child. So you can do things like wrap a paginated list in a fragment and have each child automatically observed as they commit in.
Unlike, the event listeners though, we don't
unobservewhen a child is removed. If a removed child is currently intersecting, the observer callback will be called when it is removed with an empty rect. This lets you track all the currently intersecting elements by setting state from the observer callback and either adding or removing them from your list depending on the intersecting state. If you want to track the removal of items offscreen, you'd have to maintain that state separately and append intersecting data to it in the observer callback. This is what the fixture example does.There could be more convenient ways of managing the state of multiple child intersections, but basic examples are able to be modeled with the simple implementation. Let's see how the usage goes as we integrate this with more advanced loggers and other features.
For now you can only attach one observer to an instance. This could change based on usage but the fragments are composable and could be stacked as one way to apply multiple observers to the same elements.
In practice, one pattern we expect to enable is more composable logging such as
where
ImpressionLoggerwould set up the IntersectionObserver using a fragment ref with the required business logic and various components could layer it wherever the logging is needed. Currently most callsites use a hook form, which can require wiring up refs through the tree and merging refs for multiple loggers.