Skip to content

Commit b4b36ed

Browse files
arturovtpkozlowski-opensource
arturovt
authored and
pkozlowski-opensource
committed
refactor(common): tree-shake PreloadLinkCreator for client bundles (#59431)
In this commit, we tree-shake the `PreloadLinkCreator` for client bundles because it's targeting only server code. We use the pending tasks service to contribute to app stability by waiting for the module to load. PR Close #59431
1 parent d6e78c0 commit b4b36ed

2 files changed

Lines changed: 157 additions & 130 deletions

File tree

‎packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
numberAttribute,
1818
OnChanges,
1919
OnInit,
20-
PLATFORM_ID,
2120
Renderer2,
2221
SimpleChanges,
2322
ɵformatRuntimeError as formatRuntimeError,
@@ -34,7 +33,6 @@ import {
3433
} from '@angular/core';
3534

3635
import {RuntimeErrorCode} from '../../errors';
37-
import {isPlatformServer} from '../../platform_id';
3836

3937
import {imgDirectiveDetails} from './error_helper';
4038
import {cloudinaryLoaderInfo} from './image_loaders/cloudinary_loader';
@@ -290,8 +288,6 @@ export class NgOptimizedImage implements OnInit, OnChanges {
290288
private renderer = inject(Renderer2);
291289
private imgElement: HTMLImageElement = inject(ElementRef).nativeElement;
292290
private injector = inject(Injector);
293-
private readonly isServer = isPlatformServer(inject(PLATFORM_ID));
294-
private readonly preloadLinkCreator = inject(PreloadLinkCreator);
295291

296292
// An LCP image observer should be injected only in development mode.
297293
// Do not assign it to `null` to avoid having a redundant property in the production bundle.
@@ -468,7 +464,7 @@ export class NgOptimizedImage implements OnInit, OnChanges {
468464
const checker = this.injector.get(PreconnectLinkChecker);
469465
checker.assertPreconnect(this.getRewrittenSrc(), this.ngSrc);
470466

471-
if (!this.isServer) {
467+
if (typeof ngServerMode !== 'undefined' && !ngServerMode) {
472468
const applicationRef = this.injector.get(ApplicationRef);
473469
assetPriorityCountBelowThreshold(applicationRef);
474470
}
@@ -517,8 +513,9 @@ export class NgOptimizedImage implements OnInit, OnChanges {
517513
}
518514
}
519515

520-
if (this.isServer && this.priority) {
521-
this.preloadLinkCreator.createPreloadLinkTag(
516+
if (typeof ngServerMode !== 'undefined' && ngServerMode && this.priority) {
517+
const preloadLinkCreator = this.injector.get(PreloadLinkCreator);
518+
preloadLinkCreator.createPreloadLinkTag(
522519
this.renderer,
523520
this.getRewrittenSrc(),
524521
rewrittenSrcset,
@@ -557,7 +554,12 @@ export class NgOptimizedImage implements OnInit, OnChanges {
557554
}
558555
}
559556

560-
if (ngDevMode && changes['placeholder']?.currentValue && !this.isServer) {
557+
if (
558+
ngDevMode &&
559+
changes['placeholder']?.currentValue &&
560+
typeof ngServerMode !== 'undefined' &&
561+
!ngServerMode
562+
) {
561563
assertPlaceholderDimensions(this, this.imgElement);
562564
}
563565
}

‎packages/common/test/directives/ng_optimized_image_spec.ts‎

Lines changed: 147 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -36,134 +36,144 @@ import {PRECONNECT_CHECK_BLOCKLIST} from '../../src/directives/ng_optimized_imag
3636

3737
describe('Image directive', () => {
3838
describe('preload <link> element on a server', () => {
39-
it('should create `<link>` element when the image priority attr is true', () => {
40-
// Only run this test in a browser since the Node-based DOM mocks don't
41-
// allow to override `HTMLImageElement.prototype.setAttribute` easily.
42-
if (!isBrowser) return;
43-
44-
const src = 'preload1/img.png';
45-
46-
setupTestingModule({
47-
extraProviders: [
48-
{provide: PLATFORM_ID, useValue: PLATFORM_SERVER_ID},
49-
{
50-
provide: IMAGE_LOADER,
51-
useValue: (config: ImageLoaderConfig) =>
52-
config.width
53-
? `https://angular.io/${config.src}?width=${config.width}`
54-
: `https://angular.io/${config.src}`,
55-
},
56-
],
39+
describe('server', () => {
40+
beforeEach(() => {
41+
globalThis['ngServerMode'] = true;
5742
});
5843

59-
const template = `<img ngSrc="${src}" width="150" height="50" priority sizes="10vw" ngSrcset="100w">`;
60-
TestBed.overrideComponent(TestComponent, {set: {template: template}});
44+
afterEach(() => {
45+
globalThis['ngServerMode'] = undefined;
46+
});
6147

62-
const _document = TestBed.inject(DOCUMENT);
63-
const _window = _document.defaultView!;
64-
const setAttributeSpy = spyOn(
65-
_window.HTMLLinkElement.prototype,
66-
'setAttribute',
67-
).and.callThrough();
48+
it('should create `<link>` element when the image priority attr is true', () => {
49+
// Only run this test in a browser since the Node-based DOM mocks don't
50+
// allow to override `HTMLImageElement.prototype.setAttribute` easily.
51+
if (!isBrowser) return;
6852

69-
const fixture = TestBed.createComponent(TestComponent);
70-
fixture.detectChanges();
53+
const src = 'preload1/img.png';
7154

72-
const head = _document.head;
55+
setupTestingModule({
56+
extraProviders: [
57+
{provide: PLATFORM_ID, useValue: PLATFORM_SERVER_ID},
58+
{
59+
provide: IMAGE_LOADER,
60+
useValue: (config: ImageLoaderConfig) =>
61+
config.width
62+
? `https://angular.io/${config.src}?width=${config.width}`
63+
: `https://angular.io/${config.src}`,
64+
},
65+
],
66+
});
7367

74-
const rewrittenSrc = `https://angular.io/${src}`;
68+
const template = `<img ngSrc="${src}" width="150" height="50" priority sizes="10vw" ngSrcset="100w">`;
69+
TestBed.overrideComponent(TestComponent, {set: {template: template}});
7570

76-
const preloadLink = head.querySelector(`link[href="${rewrittenSrc}"]`);
71+
const _document = TestBed.inject(DOCUMENT);
72+
const _window = _document.defaultView!;
73+
const setAttributeSpy = spyOn(
74+
_window.HTMLLinkElement.prototype,
75+
'setAttribute',
76+
).and.callThrough();
7777

78-
expect(preloadLink).toBeTruthy();
78+
const fixture = TestBed.createComponent(TestComponent);
79+
fixture.detectChanges();
7980

80-
const [name, value] = setAttributeSpy.calls.argsFor(0);
81+
const head = _document.head;
8182

82-
expect(name).toEqual('as');
83-
expect(value).toEqual('image');
83+
const rewrittenSrc = `https://angular.io/${src}`;
8484

85-
expect(preloadLink!.getAttribute('rel')).toEqual('preload');
86-
expect(preloadLink!.getAttribute('as')).toEqual('image');
87-
expect(preloadLink!.getAttribute('imagesizes')).toEqual('10vw');
88-
expect(preloadLink!.getAttribute('imagesrcset')).toEqual(`${rewrittenSrc}?width=100 100w`);
89-
expect(preloadLink!.getAttribute('fetchpriority')).toEqual('high');
85+
const preloadLink = head.querySelector(`link[href="${rewrittenSrc}"]`);
9086

91-
preloadLink!.remove();
92-
});
87+
expect(preloadLink).toBeTruthy();
9388

94-
it('should not create a preload `<link>` element when src is already preloaded.', () => {
95-
// Only run this test in a browser since the Node-based DOM mocks don't
96-
// allow to override `HTMLImageElement.prototype.setAttribute` easily.
97-
if (!isBrowser) return;
89+
const [name, value] = setAttributeSpy.calls.argsFor(0);
9890

99-
const src = `preload2/img.png`;
91+
expect(name).toEqual('as');
92+
expect(value).toEqual('image');
10093

101-
const rewrittenSrc = `https://angular.io/${src}`;
94+
expect(preloadLink!.getAttribute('rel')).toEqual('preload');
95+
expect(preloadLink!.getAttribute('as')).toEqual('image');
96+
expect(preloadLink!.getAttribute('imagesizes')).toEqual('10vw');
97+
expect(preloadLink!.getAttribute('imagesrcset')).toEqual(`${rewrittenSrc}?width=100 100w`);
98+
expect(preloadLink!.getAttribute('fetchpriority')).toEqual('high');
10299

103-
setupTestingModule({
104-
extraProviders: [
105-
{provide: PLATFORM_ID, useValue: PLATFORM_SERVER_ID},
106-
{
107-
provide: IMAGE_LOADER,
108-
useValue: (config: ImageLoaderConfig) => `https://angular.io/${config.src}`,
109-
},
110-
],
100+
preloadLink!.remove();
111101
});
112102

113-
const template = `<img ngSrc="${src}" width="150" height="50" priority><img ngSrc="${src}" width="150" height="50" priority>`;
114-
TestBed.overrideComponent(TestComponent, {set: {template: template}});
103+
it('should not create a preload `<link>` element when src is already preloaded.', () => {
104+
// Only run this test in a browser since the Node-based DOM mocks don't
105+
// allow to override `HTMLImageElement.prototype.setAttribute` easily.
106+
if (!isBrowser) return;
115107

116-
const _document = TestBed.inject(DOCUMENT);
108+
const src = `preload2/img.png`;
117109

118-
const fixture = TestBed.createComponent(TestComponent);
119-
fixture.detectChanges();
110+
const rewrittenSrc = `https://angular.io/${src}`;
120111

121-
const head = _document.head;
112+
setupTestingModule({
113+
extraProviders: [
114+
{provide: PLATFORM_ID, useValue: PLATFORM_SERVER_ID},
115+
{
116+
provide: IMAGE_LOADER,
117+
useValue: (config: ImageLoaderConfig) => `https://angular.io/${config.src}`,
118+
},
119+
],
120+
});
122121

123-
const preloadImages = TestBed.inject(PRELOADED_IMAGES);
122+
const template = `<img ngSrc="${src}" width="150" height="50" priority><img ngSrc="${src}" width="150" height="50" priority>`;
123+
TestBed.overrideComponent(TestComponent, {set: {template: template}});
124124

125-
expect(preloadImages.has(rewrittenSrc)).toBeTruthy();
125+
const _document = TestBed.inject(DOCUMENT);
126126

127-
const preloadLinks = head.querySelectorAll(`link[href="${rewrittenSrc}"]`);
127+
const fixture = TestBed.createComponent(TestComponent);
128+
fixture.detectChanges();
128129

129-
expect(preloadLinks.length).toEqual(1);
130+
const head = _document.head;
130131

131-
preloadLinks[0]!.remove();
132-
});
132+
const preloadImages = TestBed.inject(PRELOADED_IMAGES);
133133

134-
it('should error when the number of preloaded images is larger than the limit', () => {
135-
// Only run this test in a browser since the Node-based DOM mocks don't
136-
// allow to override `HTMLImageElement.prototype.setAttribute` easily.
137-
if (!isBrowser) return;
134+
expect(preloadImages.has(rewrittenSrc)).toBeTruthy();
138135

139-
setupTestingModule({
140-
extraProviders: [
141-
{provide: PLATFORM_ID, useValue: PLATFORM_SERVER_ID},
142-
{
143-
provide: IMAGE_LOADER,
144-
useValue: (config: ImageLoaderConfig) => `https://angular.io/${config.src}`,
145-
},
146-
],
136+
const preloadLinks = head.querySelectorAll(`link[href="${rewrittenSrc}"]`);
137+
138+
expect(preloadLinks.length).toEqual(1);
139+
140+
preloadLinks[0]!.remove();
147141
});
148142

149-
const template = `
150-
<img ngSrc="preloaderror2/img.png" width="150" height="50" priority>
151-
<img ngSrc="preloaderror3/img.png" width="150" height="50" priority>
152-
<img ngSrc="preloaderro4/img.png" width="150" height="50" priority>
153-
<img ngSrc="preloaderror5/img.png" width="150" height="50" priority>
154-
<img ngSrc="preloaderror6/img.png" width="150" height="50" priority>
155-
<img ngSrc="preloaderror7/img.png" width="150" height="50" priority>
156-
<img ngSrc="preloaderror8/img.png" width="150" height="50" priority>
157-
<img ngSrc="preloaderror9/img.png" width="150" height="50" priority>
158-
<img ngSrc="preloaderror10/img.png" width="150" height="50" priority>
159-
`;
143+
it('should error when the number of preloaded images is larger than the limit', () => {
144+
// Only run this test in a browser since the Node-based DOM mocks don't
145+
// allow to override `HTMLImageElement.prototype.setAttribute` easily.
146+
if (!isBrowser) return;
160147

161-
expect(() => {
162-
const fixture = createTestComponent(template);
163-
fixture.detectChanges();
164-
}).toThrowError(
165-
'NG02961: The `NgOptimizedImage` directive has detected that more than 5 images were marked as priority. This might negatively affect an overall performance of the page. To fix this, remove the "priority" attribute from images with less priority.',
166-
);
148+
setupTestingModule({
149+
extraProviders: [
150+
{provide: PLATFORM_ID, useValue: PLATFORM_SERVER_ID},
151+
{
152+
provide: IMAGE_LOADER,
153+
useValue: (config: ImageLoaderConfig) => `https://angular.io/${config.src}`,
154+
},
155+
],
156+
});
157+
158+
const template = `
159+
<img ngSrc="preloaderror2/img.png" width="150" height="50" priority>
160+
<img ngSrc="preloaderror3/img.png" width="150" height="50" priority>
161+
<img ngSrc="preloaderro4/img.png" width="150" height="50" priority>
162+
<img ngSrc="preloaderror5/img.png" width="150" height="50" priority>
163+
<img ngSrc="preloaderror6/img.png" width="150" height="50" priority>
164+
<img ngSrc="preloaderror7/img.png" width="150" height="50" priority>
165+
<img ngSrc="preloaderror8/img.png" width="150" height="50" priority>
166+
<img ngSrc="preloaderror9/img.png" width="150" height="50" priority>
167+
<img ngSrc="preloaderror10/img.png" width="150" height="50" priority>
168+
`;
169+
170+
expect(() => {
171+
const fixture = createTestComponent(template);
172+
fixture.detectChanges();
173+
}).toThrowError(
174+
'NG02961: The `NgOptimizedImage` directive has detected that more than 5 images were marked as priority. This might negatively affect an overall performance of the page. To fix this, remove the "priority" attribute from images with less priority.',
175+
);
176+
});
167177
});
168178

169179
it('should not hit max preload limit when not on the server', () => {
@@ -878,6 +888,9 @@ describe('Image directive', () => {
878888
it(
879889
'should log a warning if the priority attribute is used too often',
880890
withHead('<link rel="preconnect" href="https://angular.io/">', async () => {
891+
// This test is running both on server and in the browser.
892+
globalThis['ngServerMode'] = !isBrowser;
893+
881894
// We need to reset the count as previous test might have incremented it already
882895
resetImagePriorityCount();
883896

@@ -922,6 +935,8 @@ describe('Image directive', () => {
922935
// The warning is only logged on browsers
923936
expect(consoleWarnSpy.calls.count()).toBe(0);
924937
}
938+
939+
globalThis['ngServerMode'] = undefined;
925940
}),
926941
);
927942
});
@@ -1341,36 +1356,46 @@ describe('Image directive', () => {
13411356
});
13421357

13431358
if (isBrowser) {
1344-
it('should throw if the placeholder height exceeds the threshold', () => {
1345-
setUpModuleNoLoader();
1359+
describe('browser', () => {
1360+
beforeEach(() => {
1361+
globalThis['ngServerMode'] = false;
1362+
});
1363+
1364+
afterEach(() => {
1365+
globalThis['ngServerMode'] = undefined;
1366+
});
13461367

1347-
const template = `<img ngSrc="path/img.png" width="100" height="100" style="width:1001px; height: 300px" placeholder="data:image/png;base64,${'a'.repeat(
1348-
100,
1349-
)}">`;
1368+
it('should throw if the placeholder height exceeds the threshold', () => {
1369+
setUpModuleNoLoader();
13501370

1351-
const consoleWarnSpy = spyOn(console, 'warn');
1352-
const fixture = createTestComponent(template);
1353-
fixture.detectChanges();
1354-
expect(consoleWarnSpy.calls.count()).toBe(1);
1355-
expect(consoleWarnSpy.calls.argsFor(0)[0]).toMatch(
1356-
new RegExp(`NG0${RuntimeErrorCode.PLACEHOLDER_DIMENSION_LIMIT_EXCEEDED}:`),
1357-
);
1358-
});
1371+
const template = `<img ngSrc="path/img.png" width="100" height="100" style="width:1001px; height: 300px" placeholder="data:image/png;base64,${'a'.repeat(
1372+
100,
1373+
)}">`;
13591374

1360-
it('should throw if the placeholder width exceeds the threshold', () => {
1361-
setUpModuleNoLoader();
1375+
const consoleWarnSpy = spyOn(console, 'warn');
1376+
const fixture = createTestComponent(template);
1377+
fixture.detectChanges();
1378+
expect(consoleWarnSpy.calls.count()).toBe(1);
1379+
expect(consoleWarnSpy.calls.argsFor(0)[0]).toMatch(
1380+
new RegExp(`NG0${RuntimeErrorCode.PLACEHOLDER_DIMENSION_LIMIT_EXCEEDED}:`),
1381+
);
1382+
});
13621383

1363-
const template = `<img ngSrc="path/img.png" width="100" height="100" style="height:1001px; width: 300px" placeholder="data:image/png;base64,${'a'.repeat(
1364-
100,
1365-
)}">`;
1384+
it('should throw if the placeholder width exceeds the threshold', () => {
1385+
setUpModuleNoLoader();
13661386

1367-
const consoleWarnSpy = spyOn(console, 'warn');
1368-
const fixture = createTestComponent(template);
1369-
fixture.detectChanges();
1370-
expect(consoleWarnSpy.calls.count()).toBe(1);
1371-
expect(consoleWarnSpy.calls.argsFor(0)[0]).toMatch(
1372-
new RegExp(`NG0${RuntimeErrorCode.PLACEHOLDER_DIMENSION_LIMIT_EXCEEDED}:`),
1373-
);
1387+
const template = `<img ngSrc="path/img.png" width="100" height="100" style="height:1001px; width: 300px" placeholder="data:image/png;base64,${'a'.repeat(
1388+
100,
1389+
)}">`;
1390+
1391+
const consoleWarnSpy = spyOn(console, 'warn');
1392+
const fixture = createTestComponent(template);
1393+
fixture.detectChanges();
1394+
expect(consoleWarnSpy.calls.count()).toBe(1);
1395+
expect(consoleWarnSpy.calls.argsFor(0)[0]).toMatch(
1396+
new RegExp(`NG0${RuntimeErrorCode.PLACEHOLDER_DIMENSION_LIMIT_EXCEEDED}:`),
1397+
);
1398+
});
13741399
});
13751400
}
13761401
});

0 commit comments

Comments
 (0)