Skip to content
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

fix(common): warn if using supported CDN but not built-in loader #47330

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions goldens/public-api/common/errors.md
Expand Up @@ -15,6 +15,8 @@ export const enum RuntimeErrorCode {
// (undocumented)
LCP_IMG_MISSING_PRIORITY = 2955,
// (undocumented)
MISSING_BUILTIN_LOADER = 2962,
// (undocumented)
NG_FOR_MISSING_DIFFER = -2200,
// (undocumented)
OVERSIZED_IMAGE = 2960,
Expand Down
Expand Up @@ -6,7 +6,23 @@
* found in the LICENSE file at https://angular.io/license
*/

import {createImageLoader, ImageLoaderConfig} from './image_loader';
import {createImageLoader, ImageLoaderConfig, ImageLoaderInfo} from './image_loader';

/**
* Name and URL tester for Cloudinary.
*/
export const cloudinaryLoaderInfo: ImageLoaderInfo = {
name: 'Cloudinary',
testUrl: isCloudinaryUrl
};

const CLOUDINARY_LOADER_REGEX = /https?\:\/\/[^\/]+\.cloudinary\.com\/.+/;
/**
* Tests whether a URL is from Cloudinary CDN.
*/
function isCloudinaryUrl(url: string): boolean {
return CLOUDINARY_LOADER_REGEX.test(url);
}

/**
* Function that generates an ImageLoader for Cloudinary and turns it into an Angular provider.
Expand Down
Expand Up @@ -46,7 +46,15 @@ export type ImageLoader = (config: ImageLoaderConfig) => string;
* @see `ImageLoader`
* @see `NgOptimizedImage`
*/
const noopImageLoader = (config: ImageLoaderConfig) => config.src;
export const noopImageLoader = (config: ImageLoaderConfig) => config.src;

/**
* Metadata about the image loader.
*/
export type ImageLoaderInfo = {
name: string,
testUrl: (url: string) => boolean
};

/**
* Injection token that configures the image loader function.
Expand Down
Expand Up @@ -6,7 +6,23 @@
* found in the LICENSE file at https://angular.io/license
*/

import {createImageLoader, ImageLoaderConfig} from './image_loader';
import {createImageLoader, ImageLoaderConfig, ImageLoaderInfo} from './image_loader';

/**
* Name and URL tester for ImageKit.
*/
export const imageKitLoaderInfo: ImageLoaderInfo = {
name: 'ImageKit',
testUrl: isImageKitUrl
};

const IMAGE_KIT_LOADER_REGEX = /https?\:\/\/[^\/]+\.imagekit\.io\/.+/;
/**
* Tests whether a URL is from ImageKit CDN.
*/
function isImageKitUrl(url: string): boolean {
return IMAGE_KIT_LOADER_REGEX.test(url);
}

/**
* Function that generates an ImageLoader for ImageKit and turns it into an Angular provider.
Expand Down
Expand Up @@ -6,7 +6,23 @@
* found in the LICENSE file at https://angular.io/license
*/

import {createImageLoader, ImageLoaderConfig} from './image_loader';
import {createImageLoader, ImageLoaderConfig, ImageLoaderInfo} from './image_loader';

/**
* Name and URL tester for Imgix.
*/
export const imgixLoaderInfo: ImageLoaderInfo = {
name: 'Imgix',
testUrl: isImgixUrl
};

const IMGIX_LOADER_REGEX = /https?\:\/\/[^\/]+\.imgix\.net\/.+/;
/**
* Tests whether a URL is from Imgix CDN.
*/
function isImgixUrl(url: string): boolean {
return IMGIX_LOADER_REGEX.test(url);
}

/**
* Function that generates an ImageLoader for Imgix and turns it into an Angular provider.
Expand Down
Expand Up @@ -12,7 +12,10 @@ import {RuntimeErrorCode} from '../../errors';
import {isPlatformServer} from '../../platform_id';

import {imgDirectiveDetails} from './error_helper';
import {IMAGE_LOADER} from './image_loaders/image_loader';
import {cloudinaryLoaderInfo} from './image_loaders/cloudinary_loader';
import {IMAGE_LOADER, ImageLoader, noopImageLoader} from './image_loaders/image_loader';
import {imageKitLoaderInfo} from './image_loaders/imagekit_loader';
import {imgixLoaderInfo} from './image_loaders/imgix_loader';
import {LCPImageObserver} from './lcp_image_observer';
import {PreconnectLinkChecker} from './preconnect_link_checker';
import {PreloadLinkCreator} from './preload-link-creator';
Expand Down Expand Up @@ -72,6 +75,9 @@ const ASPECT_RATIO_TOLERANCE = .1;
*/
const OVERSIZED_IMAGE_TOLERANCE = 1000;

/** Info about built-in loaders we can test for. */
export const BUILT_IN_LOADERS = [imgixLoaderInfo, imageKitLoaderInfo, cloudinaryLoaderInfo];

/**
* A configuration object for the NgOptimizedImage directive. Contains:
* - breakpoints: An array of integer breakpoints used to generate
Expand Down Expand Up @@ -385,6 +391,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
if (!this.ngSrcset) {
assertNoComplexSizes(this);
}
assertNotMissingBuiltInLoader(this.ngSrc, this.imageLoader);
if (this.priority) {
const checker = this.injector.get(PreconnectLinkChecker);
checker.assertPreconnect(this.getRewrittenSrc(), this.ngSrc);
Expand Down Expand Up @@ -873,3 +880,35 @@ function assertValidLoadingInput(dir: NgOptimizedImage) {
`To fix this, provide a valid value ("lazy", "eager", or "auto").`);
}
}

/**
* Warns if NOT using a loader (falling back to the generic loader) and
* the image appears to be hosted on one of the image CDNs for which
* we do have a built-in image loader. Suggests switching to the
* built-in loader.
*
* @param ngSrc Value of the ngSrc attribute
* @param imageLoader ImageLoader provided
*/
function assertNotMissingBuiltInLoader(ngSrc: string, imageLoader: ImageLoader) {
if (imageLoader === noopImageLoader) {
let builtInLoaderName = '';
for (const loader of BUILT_IN_LOADERS) {
if (loader.testUrl(ngSrc)) {
builtInLoaderName = loader.name;
break;
}
}
if (builtInLoaderName) {
console.warn(formatRuntimeError(
RuntimeErrorCode.MISSING_BUILTIN_LOADER,
`NgOptimizedImage: It looks like your images may be hosted on the ` +
`${builtInLoaderName} CDN, but your app is not using Angular's ` +
`built-in loader for that CDN. We recommend switching to use ` +
`the built-in by calling \`provide${builtInLoaderName}Loader()\` ` +
`in your \`providers\` and passing it your instance's base URL. ` +
`If you don't want to use the built-in loader, define a custom ` +
`loader function using IMAGE_LOADER to silence this warning.`));
}
}
}
1 change: 1 addition & 0 deletions packages/common/src/errors.ts
Expand Up @@ -32,4 +32,5 @@ export const enum RuntimeErrorCode {
INVALID_LOADER_ARGUMENTS = 2959,
OVERSIZED_IMAGE = 2960,
TOO_MANY_PRELOADED_IMAGES = 2961,
MISSING_BUILTIN_LOADER = 2962,
}
60 changes: 60 additions & 0 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -1097,6 +1097,56 @@ describe('Image directive', () => {
expect(img.src).toBe(`${IMG_BASE_URL}/img.png`);
});

it('should warn if there is no image loader but using Imgix URL', () => {
setUpModuleNoLoader();

const template = `<img ngSrc="https://some.imgix.net/img.png" width="100" height="50">`;
const fixture = createTestComponent(template);
const consoleWarnSpy = spyOn(console, 'warn');
fixture.detectChanges();

expect(consoleWarnSpy.calls.count()).toBe(1);
expect(consoleWarnSpy.calls.argsFor(0)[0])
.toMatch(/your images may be hosted on the Imgix CDN/);
});

it('should warn if there is no image loader but using ImageKit URL', () => {
setUpModuleNoLoader();

const template = `<img ngSrc="https://some.imagekit.io/img.png" width="100" height="50">`;
const fixture = createTestComponent(template);
const consoleWarnSpy = spyOn(console, 'warn');
fixture.detectChanges();

expect(consoleWarnSpy.calls.count()).toBe(1);
expect(consoleWarnSpy.calls.argsFor(0)[0])
.toMatch(/your images may be hosted on the ImageKit CDN/);
});

it('should warn if there is no image loader but using Cloudinary URL', () => {
setUpModuleNoLoader();

const template = `<img ngSrc="https://some.cloudinary.com/img.png" width="100" height="50">`;
const fixture = createTestComponent(template);
const consoleWarnSpy = spyOn(console, 'warn');
fixture.detectChanges();

expect(consoleWarnSpy.calls.count()).toBe(1);
expect(consoleWarnSpy.calls.argsFor(0)[0])
.toMatch(/your images may be hosted on the Cloudinary CDN/);
});

it('should NOT warn if there is a custom loader but using CDN URL', () => {
setupTestingModule();

const template = `<img ngSrc="https://some.cloudinary.com/img.png" width="100" height="50">`;
const fixture = createTestComponent(template);
const consoleWarnSpy = spyOn(console, 'warn');
fixture.detectChanges();

expect(consoleWarnSpy.calls.count()).toBe(0);
});

it('should set `src` using the image loader provided via the `IMAGE_LOADER` token to compose src URL',
() => {
const imageLoader = (config: ImageLoaderConfig) => `${IMG_BASE_URL}/${config.src}`;
Expand Down Expand Up @@ -1526,6 +1576,16 @@ function setupTestingModule(config?: {
});
}

// Same as above but explicitly doesn't provide a custom loader,
// so the noopImageLoader should be used.
function setUpModuleNoLoader() {
TestBed.configureTestingModule({
declarations: [TestComponent],
imports: [CommonModule, NgOptimizedImage],
providers: [{provide: DOCUMENT, useValue: window.document}]
});
}

function createTestComponent(template: string): ComponentFixture<TestComponent> {
return TestBed.overrideComponent(TestComponent, {set: {template: template}})
.createComponent(TestComponent);
Expand Down