feat(common): make the warning for lazy-loaded lcp image an error#51748
Closed
atcastle wants to merge 1 commit intoangular:mainfrom
Closed
feat(common): make the warning for lazy-loaded lcp image an error#51748atcastle wants to merge 1 commit intoangular:mainfrom
atcastle wants to merge 1 commit intoangular:mainfrom
Conversation
AndrewKushnir
approved these changes
Sep 13, 2023
819291b to
034cd1e
Compare
034cd1e to
424f077
Compare
AndrewKushnir
approved these changes
Sep 15, 2023
424f077 to
8d997f9
Compare
Contributor
|
Caretaker note: presubmit was "green" prior to rebase. There were no changes to the TS files during the rebase. |
Member
|
This PR was merged into the repository by commit fe2fd7e. |
pkozlowski-opensource
added a commit
to pkozlowski-opensource/angular
that referenced
this pull request
Sep 18, 2023
…rror (angular#51748)" This reverts commit fe2fd7e.
pkozlowski-opensource
added a commit
that referenced
this pull request
Sep 18, 2023
Contributor
|
TGP. |
8d997f9 to
ff7dd58
Compare
Contributor
AndrewKushnir
left a comment
There was a problem hiding this comment.
@atcastle looks good, thanks! Just left a few comments.
A couple additional notes:
- we should remove the "BREAKING CHANGE" part from the commit message - we are not changing the behavior in this PR. We'll add the message in a followup PR where we flip the default value.
- it'd be great to add some tests into
packages/core/test/test_bed_spec.ts(see existing tests for similarerrorOnconfig options). - we can also add some tests to the
packages/common/test/directives/ng_optimized_image_spec.tsfile to verify correct behavior (throw vs console.warn depending on a setting)
packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts
Outdated
Show resolved
Hide resolved
packages/common/src/directives/ng_optimized_image/lcp_image_observer.ts
Outdated
Show resolved
Hide resolved
ff7dd58 to
5b2d1d7
Compare
…y an error add logic to upgrade lazy-load error to warning, if a test bed option is set
5b2d1d7 to
22e192a
Compare
Contributor
|
TGP. |
Contributor
|
@atcastle FYI the TGP is "green" for this PR. |
Contributor
|
Closing this PR in favor of #52004. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
ChellappanRajan
pushed a commit
to ChellappanRajan/angular
that referenced
this pull request
Jan 23, 2024
…gular#51748) upgrade the warning for lazy-loaded lcp images when using NgOptimizedImage to an error BREAKING CHANGE: Previously when NgOptimizedImage directive detected that an LCP image is lazy-loaded, a console warning was produced. Now the directive throws an error to make it more discoverable in a console. If you receive this error, refer to this guide for additional information: https://angular.io/guide/image-directive#step-4-mark-images-as-priority PR Close angular#51748
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
This PR upgrades on the of the NgOptimizedImage warnings to an error. The warning upgraded is the one that fires when you don't use the
priorityattribute on your LCP image, which will cause it to lazy-load and can hurt LCP performance.This error is only present when the application is run in dev mode. Because the error is thrown outside of the page rendering process, it is logged in the console as an error, but does not interrupt browsing the application. CC: @kara @AndrewKushnir