Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix PerformanceObserver usage for older browsers and CI #30387

Merged
merged 2 commits into from
Oct 27, 2021
Merged

Fix PerformanceObserver usage for older browsers and CI #30387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a873f98
Fix PerformanceObserver usage for older browsers
styfle Oct 27, 2021
a4bbabb
Remove unused code snippet
styfle Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions docs/basic-features/image-optimization.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,7 @@ Loaders can be defined per-image, or at the application level.

You should add the `priority` property to the image that will be the [Largest Contentful Paint (LCP) element](https://web.dev/lcp/#what-elements-are-considered) for each page. Doing so allows Next.js to specially prioritize the image for loading (e.g. through preload tags or priority hints), leading to a meaningful boost in LCP.

The LCP element is typically the largest image or text block visible within the viewport of the page. You can verify which element this is by running the following code in the console of your page and looking at the latest result:

```javascript
new PerformanceObserver((entryList) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than deleting this helper entirely, it might make sense if we clarify that it will only work with newer browsers? I still think it would be a helpful tool for folks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kara We don't need to document PerformanceObserver usage because Next.js will call PerformanceObserver automatically. See packages/next/client/image.tsx

Maybe I misunderstood your comment though?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's useful even with the warning. The warning tells you that you did something wrong after the fact. The docs should tell you how to get it right when you first try it out.

Currently the developer journey is:

  • Look at docs for next/image
  • You may be confused because you don't understand how to find the LCP image
  • Try out the next/image component without priority (because you're not clear on how to use it)
  • Immediately get warned you're doing something wrong, even though you tried to follow the docs
  • Fix the priority attribute

If we add detail to the docs, we can help devs write it correctly the first time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add detail to the docs, we can help devs write it correctly the first time.

I don't think it will be correct the first time because they'll have to write the code, then copy/paste a script, then analyze the output, then add the priority prop.

The automated solution skips all of that.

Now there might be a case to document it for production apps since the warning only prints during next dev and LCP could change for a production deployment.

Maybe we should direct users to use dev tools instead which will show LCP?

Does that solve the problem?

for (const entry of entryList.getEntries()) {
console.log('LCP candidate:', entry.startTime, entry.element)
}
}).observe({ type: 'largest-contentful-paint', buffered: true })
```
The LCP element is typically the largest image or text block visible within the viewport of the page. When you run `next dev`, you'll see a console warning if the LCP element is an `<Image>` without the `priority` property.

Once you've identified the LCP image, you can add the property like this:

Expand Down
6 changes: 5 additions & 1 deletion packages/next/client/image.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,11 @@ export default function Image({
)
}

if (typeof window !== 'undefined' && !perfObserver) {
if (
typeof window !== 'undefined' &&
!perfObserver &&
window.PerformanceObserver
) {
perfObserver = new PerformanceObserver((entryList) => {
for (const entry of entryList.getEntries()) {
// @ts-ignore - missing "LargestContentfulPaint" class with "element" prop
Expand Down