-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
Add React performance warnings #239
base: master
Are you sure you want to change the base?
Conversation
@atcastle the build issue has been resolved 👍 just need to fix the typing issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happened to notice some typos while vendoring the Cloudinary SDK.
`and may slow down page load. You may address this by supplying a height and width for the image, ` + | ||
`or by using the responsive image plugin. ` + | ||
`Rendered size: ${clientWidth}x${clientHeight}. Intrinsic size: ${naturalWidth}x${naturalHeight}. ` + | ||
`This warning can be surpressed by adding the 'silence-warnings' attribute to AdvancedImage.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`This warning can be surpressed by adding the 'silence-warnings' attribute to AdvancedImage.` | |
`This warning can be suppressed by adding the 'silence-warnings' attribute to AdvancedImage.` |
`An image with URL ${imgRef.src} has ' loading="lazy"' and has also been detected to be a possible ` + | ||
`LCP element (https://web.dev/lcp). This can have a significant negative impact on page loading performance. ` + | ||
`To fix this issue, remove, 'loading="lazy"' from images which may render in the initial viewport. ` + | ||
`This warning can be surpressed by adding the 'silence-warnings' attribute to AdvancedImage.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
`This warning can be surpressed by adding the 'silence-warnings' attribute to AdvancedImage.` | |
`This warning can be suppressed by adding the 'silence-warnings' attribute to AdvancedImage.` |
Note: This PR is currently a draft and untested, because of a build issue that currently prevents tests from running in the repo. @jerzy-mankowski has said they will address it when they have time. I will revise this PR afterward with a set of passing E2E tests at that time.
This PR includes a draft of 2 new performance warnings for the React AdvancedImage element. Please see the accompanying issue for a full description of the rationale for these two warnings.
The warnings are:
warnOnOversizedImage
: Logs a warning when an image is loaded, and the intrinsic size is greater than the rendered size by a substantial margin. That margin is defined inOVERSIZE_IMAGE_TOLERANCE
and initially set to 500.warnOnLazyLCP
: Logs a warning when the PerformanceObserver API detects an image is a candidate for the LCP image but has been loaded withloading=“lazy”
.Both warnings only fire if the NODE_ENV variable is present (true in most React apps) and set to “development”. Both warnings can be silenced if the AdvancedImage element is given an attribute called “silence-warnings”.