Add React performance warnings - #239
Open
atcastle wants to merge 1 commit into
Open
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
@atcastle the build issue has been resolved 👍 just need to fix the typing issues |
Sorry, something went wrong.
lishaduck
reviewed
Feb 27, 2025
lishaduck
left a comment
Copy link
Copy Markdown
There was a problem hiding this comment.
Happened to notice some typos while vendoring the Cloudinary SDK.
Sorry, something went wrong.
Comment thread
packages/react/src/internal/PerformanceWarnings.ts
| `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.` |
Copy link
Copy Markdown
There was a problem hiding this comment.
Suggested change
| `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.` |
Sorry, something went wrong.
Comment thread
packages/react/src/internal/PerformanceWarnings.ts
| `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.` |
Copy link
Copy Markdown
There was a problem hiding this comment.
Same
Suggested change
| `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.` |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
1 more reviewer3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 in OVERSIZE_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 with loading=“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”.