Make Frontity components AMP-aware

[SantosGuillamot]

This is needed for AMP feature. For full context please check out the final Implementation proposal.

---

The AMP framework uses a special syntax for the images, the iframes and the scripts, so we have to make Frontity components AMP-aware.

We can take this opportunity to remove the Intersection Observer fallback from the Image and Iframe components, that doesn't make sense anymore.

[michalczaplinski]

We can take this opportunity to remove the Intersection Observer fallback from the Image and Iframe components, that doesn't make sense anymore.

@SantosGuillamot Could you clarify this for me? Do we want to deprecate the useInView hook and only rely on native lazy-loading or are you talking about something else?

[SantosGuillamot]

Could you clarify this for me? Do we want to deprecate the useInView hook and only rely on native lazy-loading or are you talking about something else?

I don't think we have to deprecate the useInView hook because I guess it could be useful for elements where native lazy-loading doesn't affect. I think we should remove the fallback, at least, in the Image component because it could affect the performance. I think these are the lines of code we are talking about. Maybe @luisherranz can explain if there's something missing because he knows better than me the issues the InterSectionObserver is causing right now.

[luisherranz]

Yes, useInView is used in many places, not only in <Image> and <Iframe>.

I think these are the lines of code we are talking about.

It's not only those lines, it's pretty much the whole component! 😄

Something along these lines:

const Image = ({ loading = "lazy", ...props }) => {
  const { state } = useConnect();
  if (state.frontity.mode === "amp") return <amp-img {...props} />;
  return <img {...props} loading={loading} />;
};

export default connect(Image, { injectProps: false });

The API (component props) can't change because it needs to be backward compatible.

Maybe @luisherranz can explain if there's something missing because he knows better than me the issues the InterSectionObserver is causing right now

The problem with the current component is that the image doesn't load until the hydration has finished, which means a very bad Largest Contentful Paint.

[michalczaplinski]

ok, gotcha. Thanks guys! 🙂

[michalczaplinski]

I've read through those community posts:

• https://community.frontity.org/t/image-component-avoid-layout-shift-on-image-load/866/
• https://community.frontity.org/t/html2react-breaks-images-within-figure/2674/

and there are still some things that are not clear to me.

The problem with the current component is that the image doesn't load until the hydration has finished, which means a very bad Largest Contentful Paint.

I think that the problem here is that we don't show the image until hydration:

https://www.loom.com/share/ce5cf10b85fe49609f4cc8684da58b9d

But I wonder what's the best thing to do here. I think we have 3 options:

1. Remove all the code that does the lazy loading using useInView and don't hide the image using visibility: hidden on the server. If I understand correctly, this was the option suggested by Luis:

   It's not only those lines, it's pretty much the whole component! 😄

2. Use the size information provided by Gutenberg and include the Gutenberg CSS (in case of WP images) and in case of other images either require an explicit width & height or (if those are not provided) use a default to a "fill" layout. Those are basically the steps outlined by Luis in https://community.frontity.org/t/image-component-avoid-layout-shift-on-image-load/866/30

   I think this approach is probably the best long-term, but it is going to take a lot of work. For example, I was looking at the Image element from next/image and they have introduced a layout prop which works very similarly to the layout attribute in AMP.

   https://www.loom.com/share/92b24d323bd64f64b5b8fccb2858d2d6

3. We can also not do any of the things that I've just mentioned for the moment and just add the

   const { state } = useConnect();
   if (state.frontity.mode === "amp") return <amp-img {...props} />;
   return <img {...props} loading={loading} />;

   that we need to make it work with AMP and be done with it 😃

@SantosGuillamot What do you think?

[SantosGuillamot]

Thanks for the explanation Michal 🙂 The initial idea was that, as we were going to work on the Image component for this feature, we could take the opportunity to remove the fallback of the IntersectionObserver which was causing some issues. The way I see it, I agree the option 2 would be a better solution, but as you mention it seems to be many extra work. I would go with the option 1 and treat the option 2 as a different Feature Discussion that could be address later.

[luisherranz]

I think that the problem here is that we don't show the image until hydration:
https://www.loom.com/share/ce5cf10b85fe49609f4cc8684da58b9d

It's not that much the visibility of the image but the fact that the SSR'ed <img> tag doesn't have a src and srcset attribute:

• SSR: An <img> with data-src and data-srcset. The browser doesn't start the network request because it has no src.
• Hydration: Frontity changes the data-src to src and data-srcset to srcset. The browser sees the src attribute and does the network request, then once it's finished, it shows the image.

Doing SSR with data-src is required for the Intersection Observer fallback because if you do SSR with src, the browser will start the network request for that image when the HTML is received, and therefore there's no way to lazy load it manually when it enters the viewport.

But thankfully we can remove that trickery now that native lazy load is present in modern browsers 🙂