Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(common): Fire priority check on stable. (#56776)

Browse source 
To support routing on app init, the directive will now fire the priority check when the apps become stable.

fixes #56757

PR Close #56776
This commit is contained in:
Matthieu Riegler 2024-06-30 15:20:10 +02:00 committed by Jessica Janiuk
parent 00d9cd240d
commit d9fe17a3df
2 changed files with 40 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts
View file

@ -30,6 +30,8 @@ import {
ɵSafeValue as SafeValue,
ɵunwrapSafeValue as unwrapSafeValue,
ChangeDetectorRef,
ApplicationRef,
ɵwhenStable as whenStable,
} from '@angular/core';
import {RuntimeErrorCode} from '../../errors';
@ -155,6 +157,13 @@ const PRIORITY_COUNT_THRESHOLD = 10;
*/
let IMGS_WITH_PRIORITY_ATTR_COUNT = 0;
/**
* This function is for testing purpose.
*/
export function resetImagePriorityCount() {
IMGS_WITH_PRIORITY_ATTR_COUNT = 0;
}
/**
* Config options used in rendering placeholder images.
*
@ -449,12 +458,9 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
const checker = this.injector.get(PreconnectLinkChecker);
checker.assertPreconnect(this.getRewrittenSrc(), this.ngSrc);
// This leaves the Angular zone to avoid triggering unnecessary change detection cycles when
// document.addEventListener is invoked
if (!this.isServer) {
ngZone.runOutsideAngular(() => {
assetPriorityCountBelowThreshold();
});
const applicationRef = this.injector.get(ApplicationRef);
assetPriorityCountBelowThreshold(applicationRef);
}
}
}
@ -1278,22 +1284,23 @@ function assertNoLoaderParamsWithoutLoader(dir: NgOptimizedImage, imageLoader: I
/**
* Warns if the priority attribute is used too often on page load
*/
function assetPriorityCountBelowThreshold() {
async function assetPriorityCountBelowThreshold(appRef: ApplicationRef) {
if (IMGS_WITH_PRIORITY_ATTR_COUNT === 0) {
document.addEventListener('DOMContentLoaded', () => {
if (IMGS_WITH_PRIORITY_ATTR_COUNT > PRIORITY_COUNT_THRESHOLD) {
console.warn(
formatRuntimeError(
RuntimeErrorCode.TOO_MANY_PRIORITY_ATTRIBUTES,
`NgOptimizedImage: The "priority" attribute is set to true more than ${PRIORITY_COUNT_THRESHOLD} times (${IMGS_WITH_PRIORITY_ATTR_COUNT} times). ` +
`Marking too many images as "high" priority can hurt your application's LCP (https://web.dev/lcp). ` +
`"Priority" should only be set on the image expected to be the page's LCP element.`,
),
);
}
});
IMGS_WITH_PRIORITY_ATTR_COUNT++;
await whenStable(appRef);
if (IMGS_WITH_PRIORITY_ATTR_COUNT > PRIORITY_COUNT_THRESHOLD) {
console.warn(
formatRuntimeError(
RuntimeErrorCode.TOO_MANY_PRIORITY_ATTRIBUTES,
`NgOptimizedImage: The "priority" attribute is set to true more than ${PRIORITY_COUNT_THRESHOLD} times (${IMGS_WITH_PRIORITY_ATTR_COUNT} times). ` +
`Marking too many images as "high" priority can hurt your application's LCP (https://web.dev/lcp). ` +
`"Priority" should only be set on the image expected to be the page's LCP element.`,
),
);
}
} else {
IMGS_WITH_PRIORITY_ATTR_COUNT++;
}
IMGS_WITH_PRIORITY_ATTR_COUNT++;
}
/**

22
packages/common/test/directives/ng_optimized_image_spec.ts
View file

@ -30,6 +30,7 @@ import {
NgOptimizedImage,
PLACEHOLDER_BLUR_AMOUNT,
RECOMMENDED_SRCSET_DENSITY_CAP,
resetImagePriorityCount,
} from '../../src/directives/ng_optimized_image/ng_optimized_image';
import {PRECONNECT_CHECK_BLOCKLIST} from '../../src/directives/ng_optimized_image/preconnect_link_checker';
@ -871,12 +872,16 @@ describe('Image directive', () => {
expect(img.getAttribute('fetchpriority')).toBe('auto');
});
it('should log a warning if the priority attribute is used too often', async () => {
withHead('', () => {
it(
'should log a warning if the priority attribute is used too often',
withHead('<link rel="preconnect" href="https://angular.io/">', async () => {
// We need to reset the count as previous test might have incremented it already
resetImagePriorityCount();
const imageLoader = () => {
// We need something different from the `localhost` (as we don't want to produce
// a preconnect warning for local environments).
return 'https://angular.io/assets/images/logos/angular/angular.svg';
return 'https://angular.io/assets/images/logos/path/img.png';
};
setupTestingModule({imageLoader});
@ -900,9 +905,10 @@ describe('Image directive', () => {
const fixture = createTestComponent(template);
fixture.detectChanges();
// We manually fire the event that is listened by the directive
// as it won't fire in the context of our unit test
document.dispatchEvent(new Event('DOMContentLoaded'));
await fixture.whenStable();
// trick to wait for the whenStable() to fire in the directive
await Promise.resolve();
if (isBrowser) {
expect(consoleWarnSpy.calls.count()).toBe(1);
@ -913,8 +919,8 @@ describe('Image directive', () => {
// The warning is only logged on browsers
expect(consoleWarnSpy.calls.count()).toBe(0);
}
});
});
}),
);
});
describe('meta data', () => {