From bd76ad6fa60c1c3da8a46faacdc54c1093dbac3a Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 10 Jun 2022 12:36:28 +0200 Subject: [PATCH] refactor(common): properly configure PRECONNECT_CHECK_BLOCKLIST (#47082) Refactoring tests made me realise that we are misinterpreting the ensurePreconnect option - in fact it should configure the PRECONNECT_CHECK_BLOCKLIST provider _only_ if the ensurePreconnect is set to false. PR Close #47082 --- .../image_loaders/image_loader.ts | 2 +- .../test/image_loaders/image_loader_spec.ts | 134 +++++------------- 2 files changed, 39 insertions(+), 97 deletions(-) diff --git a/packages/common/src/directives/ng_optimized_image/image_loaders/image_loader.ts b/packages/common/src/directives/ng_optimized_image/image_loaders/image_loader.ts index 47d0ed7d0b8..84120b302f9 100644 --- a/packages/common/src/directives/ng_optimized_image/image_loaders/image_loader.ts +++ b/packages/common/src/directives/ng_optimized_image/image_loaders/image_loader.ts @@ -54,7 +54,7 @@ export function createImageLoader( const providers: Provider[] = [{provide: IMAGE_LOADER, useValue: urlFn(path)}]; - if (ngDevMode && Boolean(options.ensurePreconnect) === true) { + if (ngDevMode && options.ensurePreconnect === false) { providers.push({provide: PRECONNECT_CHECK_BLOCKLIST, useValue: [path], multi: true}); } diff --git a/packages/common/test/image_loaders/image_loader_spec.ts b/packages/common/test/image_loaders/image_loader_spec.ts index f1a906ca49d..d23f94d2bfe 100644 --- a/packages/common/test/image_loaders/image_loader_spec.ts +++ b/packages/common/test/image_loaders/image_loader_spec.ts @@ -48,38 +48,6 @@ describe('Built-in image directive loaders', () => { const config = {src: 'img.png', width: 100}; expect(loader(config)).toBe(`${path}/img.png?auto=format&w=100`); }); - - describe('options', () => { - it('should configure PRECONNECT_CHECK_BLOCKLIST token by default', () => { - const path = 'https://somesite.imgix.net'; - const providers = provideImgixLoader(path); - expect(providers.length).toBe(2); - - const valueProvider = providers[1] as ValueProvider; - expect(valueProvider.multi).toBeTrue(); - expect(valueProvider.useValue).toEqual([path]); - expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); - }); - - it('should configure PRECONNECT_CHECK_BLOCKLIST when the ensurePreconnect was specified', - () => { - const path = 'https://somesite.imgix.net'; - const providers = provideImgixLoader(path, {ensurePreconnect: true}); - expect(providers.length).toBe(2); - - const valueProvider = providers[1] as ValueProvider; - expect(valueProvider.multi).toBeTrue(); - expect(valueProvider.useValue).toEqual([path]); - expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); - }); - - it('should NOT configure PRECONNECT_CHECK_BLOCKLIST disabled with the ensurePreconnect option', - () => { - const providers = - provideImgixLoader('https://somesite.imgix.net', {ensurePreconnect: false}); - expect(providers.length).toBe(1); - }); - }); }); describe('Cloudinary loader', () => { @@ -119,38 +87,6 @@ describe('Built-in image directive loaders', () => { expect(loader({src: '/img.png'})).toBe(`${path}/image/upload/f_auto,q_auto/img.png`); }); }); - - describe('options', () => { - it('should configure PRECONNECT_CHECK_BLOCKLIST token by default', () => { - const path = 'https://res.cloudinary.com/mysite'; - const providers = provideCloudinaryLoader(path); - expect(providers.length).toBe(2); - - const valueProvider = providers[1] as ValueProvider; - expect(valueProvider.multi).toBeTrue(); - expect(valueProvider.useValue).toEqual([path]); - expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); - }); - - it('should configure PRECONNECT_CHECK_BLOCKLIST when the ensurePreconnect was specified', - () => { - const path = 'https://res.cloudinary.com/mysite'; - const providers = provideCloudinaryLoader(path, {ensurePreconnect: true}); - expect(providers.length).toBe(2); - - const valueProvider = providers[1] as ValueProvider; - expect(valueProvider.multi).toBeTrue(); - expect(valueProvider.useValue).toEqual([path]); - expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); - }); - - it('should NOT configure PRECONNECT_CHECK_BLOCKLIST disabled with the ensurePreconnect option', - () => { - const providers = provideCloudinaryLoader( - 'https://res.cloudinary.com/mysite', {ensurePreconnect: false}); - expect(providers.length).toBe(1); - }); - }); }); describe('ImageKit loader', () => { @@ -188,38 +124,6 @@ describe('Built-in image directive loaders', () => { expect(loader({src: '/img.png'})).toBe(`${path}/tr:q-auto/img.png`); }); }); - - describe('options', () => { - it('should configure PRECONNECT_CHECK_BLOCKLIST token by default', () => { - const path = 'https://ik.imageengine.io/imagetest'; - const providers = provideImageKitLoader(path); - expect(providers.length).toBe(2); - - const valueProvider = providers[1] as ValueProvider; - expect(valueProvider.multi).toBeTrue(); - expect(valueProvider.useValue).toEqual([path]); - expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); - }); - - it('should configure PRECONNECT_CHECK_BLOCKLIST when the ensurePreconnect was specified', - () => { - const path = 'https://ik.imageengine.io/imagetest'; - const providers = provideImageKitLoader(path, {ensurePreconnect: true}); - expect(providers.length).toBe(2); - - const valueProvider = providers[1] as ValueProvider; - expect(valueProvider.multi).toBeTrue(); - expect(valueProvider.useValue).toEqual([path]); - expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); - }); - - it('should NOT configure PRECONNECT_CHECK_BLOCKLIST disabled with the ensurePreconnect option', - () => { - const providers = provideImageKitLoader( - 'https://ik.imageengine.io/imagetest', {ensurePreconnect: false}); - expect(providers.length).toBe(1); - }); - }); }); describe('Cloudflare loader', () => { @@ -239,6 +143,44 @@ describe('Built-in image directive loaders', () => { }); }); + describe('ensurePreconnect option', () => { + const path = 'https://some.host/base'; + + [provideImgixLoader, + provideCloudinaryLoader, + provideImageKitLoader, + provideCloudflareLoader, + ].forEach(provideImageLoader => { + it(`${provideImageLoader.name} should _not_ block-list path by default`, () => { + const providers = provideImageLoader(path); + + expect(providers.length).toBe(1); + const valueProvider = providers[0] as ValueProvider; + expect(valueProvider.provide).not.toBe(PRECONNECT_CHECK_BLOCKLIST); + }); + + it(`${provideImageLoader.name} should _not_ block-list path when ensurePreconnect is true`, + () => { + const providers = provideImageLoader(path, {ensurePreconnect: true}); + + expect(providers.length).toBe(1); + const valueProvider = providers[0] as ValueProvider; + expect(valueProvider.provide).not.toBe(PRECONNECT_CHECK_BLOCKLIST); + }); + + it('`${provideImageLoader.name} should block-list path when ensurePreconnect is false', + () => { + const providers = provideImageLoader(path, {ensurePreconnect: false}); + expect(providers.length).toBe(2); + + const valueProvider = providers[1] as ValueProvider; + expect(valueProvider.multi).toBeTrue(); + expect(valueProvider.useValue).toEqual([path]); + expect(valueProvider.provide).toBe(PRECONNECT_CHECK_BLOCKLIST); + }); + }); + }); + describe('loader utils', () => { it('should identify valid paths', () => { expect(isValidPath('https://cdn.imageprovider.com/image-test')).toBe(true);