Overlay and iFrame flicker on updating props values

Since the original video was recorded the frequency and severity of this flickering has been reduced through other work being merged in.

I'm still, on occasion able to see a single frame flash of grey when updating props (particularly if the page is long and I'm editing something towards the bottom)

It appears that it's possible for the current iframe to be hidden before the replacement iframe has rendered thus showing a single frame with neither iframe visible:

This issue, and likely many of the other issues with flickering and tearing, is directly caused by the following pattern, which is used in many places in the code and will, 100% of the time, cause flickering and tearing.

function Component({prop}) {
  const [derivedState, setDerivedState] = useState(null);

  useEffect({
    const derived = someLogic(prop);
    setDerivedState(derived);
  }, [prop]);
}

This will always lead to the following:
* 2 frames are rendered where you want 1
* The first of those frames uses the new prop state with the old derived state. In other words, it will always cause tearing by showing a wrong combination of values

At best, the frames are rendered fast enough so that the flickering is hard to notice. That's why this problem often goes unnoticed in initial phases of a project when there's little time scripting, but becomes extremely costly after some time, as each of these frames takes longer and longer to produce.

Additionally, it's very likely to result in infinite loops, as every new render has the opportunity to trigger yet another render.

Occurrences of this pattern

I did a quick pass on all usages of useEffect and listed some here that are definitely problematic (i.e. they will cause at least 1 extra frame). Some other usages may also incur additional render passes in other ways, but I'll focus on the ones that are easiest to confirm, like those involving useState. It's not an exhaustive list and may miss some important cases.

https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/ho...

Every one of these, or potentially multiple, could be what is causing this particular flickering issue. I'll update here once I have the local setup to validate which one it is.

In any case, every such occurrence will always lead to undesirable behavior. Once this pattern is replaced by a solution that doesn't render twice, you can expect huge gains in application performance, and no more tearing.

Quick fixes

In many cases it's possible to, as a temporary improvement, replace "useEffect" with "useLayoutEffect". While it will not reduce the amount of scripting by a lot, it will avoid the wrong frame, by asking React to immediately process the effect before drawing the frame. Definitely not an ideal solution but should be "plug and play" (only replacing function name with same logic and deps), and experience strictly less issues than the same pattern with useEffect.

It can also be an option to simply derive the state synchronously every time the component renders, without putting it in useState. It might feel inefficient, but the overhead is likely still a lot less than the extra React render pass, which also involves doubling the amount of style recalculation (often more expensive than scripting). For many cases this will actually perform better than switching to useLayoutEffect.

Long term solutions

This depends on how state is used in general. You want some primitives that handle this type of state management requirement nicely. But it will vary wildly depending on what type of state management solution is used.

The most important thing is that the solution allows you to do a single render pass in all cases except where you need to wait for a request. React supports plenty different ways in which that can be achieved.

Redux probably offers a solution for at least some occurrences.

Some other options:
* useMemo can sometimes help you get the same result in one pass
* creating a reducer that does more complicated things when you dispatch, so that the state immediately becomes the right one

This issue, and likely many of the other issues with flickering and tearing, is directly caused by the following pattern, which is used in many places in the code and will, 100% of the time, cause flickering and tearing.

function Component({prop}) {
  const [derivedState, setDerivedState] = useState(null);

  useEffect({
    const derived = someLogic(prop);
    setDerivedState(derived);
  }, [prop]);
}

This will always lead to the following:
* 2 frames are rendered where you want 1
* The first of those frames uses the new prop state with the old derived state. In other words, it will always cause tearing by showing a wrong combination of values

At best, the frames are rendered fast enough so that the flickering is hard to notice. That's why this problem often goes unnoticed in initial phases of a project when there's little time scripting, but becomes extremely costly after some time, as each of these frames takes longer and longer to produce.

Additionally, it's very likely to result in infinite loops, as every new render has the opportunity to trigger yet another render.

Occurrences of this pattern

I did a quick pass on all usages of useEffect and listed some here that are definitely problematic (i.e. they will cause at least 1 extra frame). Some other usages may also incur additional render passes in other ways, but I'll focus on the ones that are easiest to confirm, like those involving useState. It's not an exhaustive list and may miss some important cases.

https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/co...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/fe...
https://git.drupalcode.org/project/experience_builder/blob/0.x/ui/src/ho...

Every one of these, or potentially multiple, could be what is causing this particular flickering issue. I'll update here once I have the local setup to validate which one it is.

In any case, every such occurrence will always lead to undesirable behavior. Once this pattern is replaced by a solution that doesn't render twice, you can expect huge gains in application performance, and no more tearing.

Quick fixes

In many cases it's possible to, as a temporary improvement, replace "useEffect" with "useLayoutEffect". While it will not reduce the amount of scripting by a lot, it will avoid the wrong frame, by asking React to immediately process the effect before drawing the frame. Definitely not an ideal solution but should be "plug and play" (only replacing function name with same logic and deps), and experience strictly less issues than the same pattern with useEffect.

It can also be an option to simply derive the state synchronously every time the component renders, without putting it in useState. It might feel inefficient, but the overhead is likely still a lot less than the extra React render pass, which also involves doubling the amount of style recalculation (often more expensive than scripting). For many cases this will actually perform better than switching to useLayoutEffect.

Long term solutions

This depends on how state is used in general. You want some primitives that handle this type of state management requirement nicely. But it will vary wildly depending on what type of state management solution is used.

The most important thing is that the solution allows you to do a single render pass in all cases except where you need to wait for a request. React supports plenty different ways in which that can be achieved.

Redux probably offers a solution for at least some occurrences.

Some other options:
* useMemo can sometimes help you get the same result in one pass
* creating a reducer that does more complicated things when you dispatch, so that the state immediately becomes the right one

I hope my previous comment isn't too abstract. I'm posting already because it seems valuable to already convey the gist of the problem before spending time to figure out exactly how to solve it here.

@inwerpsel thank you so much for your incredibly in depth and detailed description and the work to identify areas of concern. In this specific case, I think the issue was caused by swapping in the iframe from display: none; to display: block; however I think you have probably identified numerous other places where performance can be improved.

Would you be able to raise a new issue with your observations but with a more general target of improving front end performance over all as I fear that this valuable report will get lost as a comment on this particular issue?

This seems to improve the flickering situation but doesn't quite remove it, at least when using in-browser code components. Here's a short gif:

@lauriii I pushed further changes that should stop the height of the iframe jumping up between renders which I think is what is happening in your gif.

Tested the latest changes here. I'm still getting flickering. It does look like it is primarily the JavaScript components that are flickering. Wondering if we are waiting long enough before swapping the iframes to have the JavaScript components render?

@jessebaker: If you start digging into #11 for JS components, here's a few ideas I can think of that might help:

• The Astro hydration library ideally should run sooner. I commented on #3509357-10: Working with grids with slots is difficult due to missing Astro CSS → about that.
• Check the iframe srcDoc for rel="modulepreload" links. Do browsers delay the iframe's load event until these are all loaded? If not, that would explain why js components might not be rendered when XB swaps the iframes. But if browsers do delay the load event until those are all preloaded, then the question is: are there any JS files being downloaded that aren't in that preload list and therefore not blocking the load event?
• Because Astro's hydration uses async functions that await on modules getting imported, even if the iframe's load event is being correctly delayed until those modules are preloaded, I wonder if there's still a timing issue where the iframe's load event is fired before all those async promises are resolved. In which case, would adding a setTimeout(0) between the iframe's load event and the swapping be enough to ensure those promises get resolved first?

Check the iframe srcDoc for rel="modulepreload" links. Do browsers delay the iframe's load event until these are all loaded? If not, that would explain why js components might not be rendered when XB swaps the iframes.

From https://html.spec.whatwg.org/multipage/links.html#link-type-modulepreload: A user agent must not delay the load event for this link type.

I think this is very likely the culprit, especially if some of the JS assets are in auto-save storage, meaning their URL needs to do a full Drupal bootstrap and isn't cacheable.

One idea for fixing #13 is to use the resource timing API. In other words, instead of swapping the iframe when the load event fires, delay the swapping until both the load event fires and also all URLs referenced by rel="modulepreload" links are in the resource timing list and have a responseEnd timestamp.

I've created a new issue to track the Code Component related issue that @lauriii is seeing. ✨ iFrame swapper needs to more intelligently wait on Code Components before swapping Active

This issue/MR is specifically targeting a React implementation issue where the React component that renders the iFrame was displaying a single frame with the incorrect height/position calculations before immediately showing the correct frame. It affected both the position of the borders in the overlay and the height of the iFrame itself. It occurs regardless having Code Components or SDCs on the page. It is/was significantly more obvious the slower your CPU.

Before: (4x CPU slow down in Chrome Dev tool enabled)

After: (4x CPU slow down in Chrome Dev tool enabled)

Thanks @bnjmnm for the test fix.

Automatically closed - issue fixed for 2 weeks with no activity.