Commits on Feb 19, 2024

1. Move all markRef calls into begin phase …

   Certain fiber types may have a ref attached to them. The main ones are
   HostComponent and ClassComponent. During the render phase, we check if
   a ref was passed to it, and if so, we schedule a Ref effect: `markRef`.
   
   Currently, we're not consistent about whether we call `markRef` in the
   begin phase or the complete phase. For some fiber types, I found that
   `markRef` was called in both phases, causing redundant work.
   
   After some investigation, I don't believe it's necessary to call
   `markRef` in both the begin phase and the complete phase, as long as you
   don't bail out before calling `markRef`.
   
   I though that maybe it had to do with the
   `attemptEarlyBailoutIfNoScheduledUpdates` branch, which is a fast path
   that skips the regular begin phase if no new props, state, or context
   were passed. But if the props haven't changed (referentially —
   the `memo` and `shouldComponentUpdate` checks happen later), then it
   follows that the ref couldn't have changed either. This is true even in
   the old `createElement` runtime where `ref` is stored on the element
   instead of as a prop, because there's no way to pass a new ref to an
   element without also passing new props. You might argue this is a
   leaky assumption, but since we're shifting ref to be just a regular prop
   anyway, I think it's the correct way to think about it going forward.
   
   I think the pattern of calling `markRef` in the complete phase may have
   been left over from an earlier iteration of the implementation before
   the bailout logic was structured like it is today.
   
   So, I removed all the `markRef` calls from the complete phase. In the
   case of ScopeComponent, which had no corresponding call in the begin
   phase, I added one.
   
   We already had a test that asserted that a ref is reattached even if
   the component bails out, but I added some new ones to be extra safe.
   
   The reason I'm changing this this is because I'm working on a different
   change to move the ref handling logic in `coerceRef` to happen in render
   phase of the  component that accepts the ref, instead of during the
   parent's reconciliation.

   

   acdlite committed Feb 19, 2024
   Configuration menu

   • View commit details
   • Copy full SHA for 0dae8b5
   • Browse repository at this point

   Copy the full SHA

   0dae8b5 View commit details

   Browse the repository at this point in the history