[ResizeObserver 3/3] ResizeObserver Web API implementation - #57723
[ResizeObserver 3/3] ResizeObserver Web API implementation#57723paradowstack wants to merge 39 commits into
Conversation
meta-cla
Bot
added
the
CLA Signed
a7e8c72 to
d9da0c1
Compare
facebook-github-tools
Bot
added
the
Shared with Meta
|
@rubennorte has imported this pull request. If you are a Meta employee, you can view this in D114045415. |
|
Are the CI failures legit? |
795d2b1 to
027b820
Compare
There were missing targets in Package.swift file - the last commit fixed that. |
rubennorte
left a comment
rubennorte
left a comment
There was a problem hiding this comment.
This looks great, thanks a lot!
Could you please address the comments in the PR?
Also, I think we should add more tests to see what would happen if a callback scheduled an update in a component. React DOM would probably batch those and run in a separate task, but they also provide flushSync to process them immediately. We don't provide that now in RN but I think we should at some point for use cases like this.
| ... | ||
| }; | ||
|
|
||
| declare type ResizeObserverEntry = { |
There was a problem hiding this comment.
Both this and ResizeObserverSize should be declared as as classes.
| // See | ||
| // https://w3c.github.io/csswg-drafts/resize-observer/#broadcast-resize-notifications-h | ||
| if (resizeObserverDelegate_ != nullptr) { | ||
| resizeObserverDelegate_->runResizeObservations(); |
There was a problem hiding this comment.
I think this is incomplete. This should be run in a loop to process updates coming callbacks themselves. Check resizeObserverDepth in https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model
| _blockSize: number; | ||
|
|
||
| constructor(inlineSize: number, blockSize: number) { | ||
| this._inlineSize = inlineSize; |
There was a problem hiding this comment.
Can we make this constructor and the one in ResizeObserverEntry private using the same pattern we're using in other Web classes (see <Class>_public)
| zeroContentRect); | ||
| } | ||
|
|
||
| auto layoutMetrics = LayoutableShadowNode::computeRelativeLayoutMetrics( |
There was a problem hiding this comment.
This call is expensive and not necessary for ResizeObserver. In ResizeObserver, the position of the element in the viewport is not considered, only its size. For that, you can just look at the own layout of the element, and don't need the bounding client rect computed from that function.
| // the padding box (i.e. the paddings only, excluding borders). | ||
| // https://w3c.github.io/csswg-drafts/resize-observer/#dom-resizeobserverentry-contentrect | ||
| auto contentRect = Rect{ | ||
| .origin = |
There was a problem hiding this comment.
This is against the spec. The origin is 0,0 for the contentRect provided.
|
Also, re: the changelog, this should just be [internal] until we promote this to the experimental/canary channels |
2 participants
Summary:
Implements the
ResizeObserverWeb API for the New Architecture, behind theenableResizeObserverByDefaultflag (off by default).The motivation is Web compatibility, and it's more capable than
onLayout: callers pick which box to observe (content-box,border-box,device-pixel-content-box) and the sizes for those boxes are delivered in the notification.The design is as follows: JS
ResizeObserver/ResizeObserverEntry/ResizeObserverSizeon top of a manager singleton and theNativeResizeObserverTurboModule, using the same notify +takeRecordspull model. In C++,ResizeObserverManagercollects observed targets whose layout changed at commit time (viashadowTreeDidCommit) and then computes and delivers observations in the event loop's "update the rendering" step (RuntimeSchedulerResizeObserverDelegate::runResizeObservations), as the spec requires. Requires bridgeless + the modern event loop; the legacy scheduler gets a no-op delegate.Known deviations from the spec (each pinned by a test):
ResizeObserver loop completed with undelivered notificationserror is never emitted; a callback that triggers a resize is delivered on the next tick.observe()algorithm.observe()order rather than construction order.Changelog:
[GENERAL] [ADDED] - Add the
ResizeObserverAPI, behind theenableResizeObserverByDefaultfeature flagTest Plan:
ResizeObserver-itest.jscovers many test scenarios.ResizeObserverexamples (box sizes, text, visibility); verified initial, resize, and animation-driven delivery there.