From 8d68b05b35edaf641860822a6dad13989a582854 Mon Sep 17 00:00:00 2001 From: Gretchen Shelby-Dormer <35184207+gshel@users.noreply.github.com> Date: Mon, 13 Nov 2023 14:51:23 -0400 Subject: [PATCH 1/3] feat: Get loadingStrategy value from netlify.toml inputs, default to 'lazy' if not provided (#98) ## Description loadingStrategy was not being read from the netlify.toml; this PR gets the value from the netlify.toml and if it doesn't exist, it defaults to `lazy`. ## Issue Ticket Number Fixes https://github.com/cloudinary-community/netlify-plugin-cloudinary/issues/97 ## Type of change - [X] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ## Checklist - [X] I have followed the contributing guidelines of this project as mentioned in [CONTRIBUTING.md](https://github.com/cloudinary-community/netlify-plugin-cloudinary/blob/main/CONTRIBUTING.md) - [X] I have created an [issue](https://github.com/colbyfayock/netlify-plugin-cloudinary/issues) ticket for this PR - [X] I have checked to ensure there aren't other open [Pull Requests](https://github.com/colbyfayock/netlify-plugin-cloudinary/pulls) for the same update/change? - [X] I have performed a self-review of my own code - [X] I have run tests locally to ensure they all pass - [X] I have commented my code, particularly in hard-to-understand areas - [X] I have made corresponding changes needed to the documentation --- netlify-plugin-cloudinary/src/index.ts | 4 +- .../src/lib/cloudinary.ts | 4 +- .../tests/lib/cloudinary-cname.test.js | 3 +- .../tests/lib/cloudinary-privatecdn.test.js | 3 +- .../tests/lib/cloudinary.test.js | 20 ++----- .../tests/on-post-build.test.js | 57 +++++++++++++++++-- 6 files changed, 68 insertions(+), 23 deletions(-) diff --git a/netlify-plugin-cloudinary/src/index.ts b/netlify-plugin-cloudinary/src/index.ts index 6242d10..fbf069c 100644 --- a/netlify-plugin-cloudinary/src/index.ts +++ b/netlify-plugin-cloudinary/src/index.ts @@ -277,7 +277,7 @@ export async function onBuild({ } mediaPaths.forEach(async mediaPath => { - const cldAssetPath = `/${path.join(PUBLIC_ASSET_PATH, mediaPath)}`; + const cldAssetPath = `/${path.posix.join(PUBLIC_ASSET_PATH, mediaPath)}`; const cldAssetUrl = `${host}${cldAssetPath}`; try { const { cloudinaryUrl: assetRedirectUrl } = await getCloudinaryUrl({ @@ -335,6 +335,7 @@ export async function onPostBuild({ cname, deliveryType, folder = process.env.SITE_NAME, + loadingStrategy = inputs.loadingStrategy || 'lazy', privateCdn, uploadPreset, } = inputs; @@ -387,6 +388,7 @@ export async function onPostBuild({ deliveryType, uploadPreset, folder, + loadingStrategy, localDir: PUBLISH_DIR, remoteHost: host, transformations diff --git a/netlify-plugin-cloudinary/src/lib/cloudinary.ts b/netlify-plugin-cloudinary/src/lib/cloudinary.ts index 38c8f15..4c7d25d 100644 --- a/netlify-plugin-cloudinary/src/lib/cloudinary.ts +++ b/netlify-plugin-cloudinary/src/lib/cloudinary.ts @@ -70,7 +70,7 @@ export type Assets = { type UpdateCloudinaryOptions = Omit & { assets: Assets; - loadingStrategy?: "lazy" + loadingStrategy: string; } /** @@ -288,7 +288,7 @@ export async function updateHtmlImagesToCloudinary(html: string, options: Update folder, localDir, remoteHost, - loadingStrategy = 'lazy', + loadingStrategy, transformations } = options diff --git a/netlify-plugin-cloudinary/tests/lib/cloudinary-cname.test.js b/netlify-plugin-cloudinary/tests/lib/cloudinary-cname.test.js index bf5d87c..52a6f56 100644 --- a/netlify-plugin-cloudinary/tests/lib/cloudinary-cname.test.js +++ b/netlify-plugin-cloudinary/tests/lib/cloudinary-cname.test.js @@ -49,7 +49,8 @@ describe('lib/util', () => { const { html } = await updateHtmlImagesToCloudinary(sourceHtml, { deliveryType: 'fetch', localDir: 'tests', - remoteHost: 'https://cloudinary.netlify.app' + remoteHost: 'https://cloudinary.netlify.app', + loadingStrategy: 'lazy' }); expect(html).toEqual(`

`); diff --git a/netlify-plugin-cloudinary/tests/lib/cloudinary-privatecdn.test.js b/netlify-plugin-cloudinary/tests/lib/cloudinary-privatecdn.test.js index 8ce2f5b..d9f095c 100644 --- a/netlify-plugin-cloudinary/tests/lib/cloudinary-privatecdn.test.js +++ b/netlify-plugin-cloudinary/tests/lib/cloudinary-privatecdn.test.js @@ -48,7 +48,8 @@ describe('lib/util', () => { const { html } = await updateHtmlImagesToCloudinary(sourceHtml, { deliveryType: 'fetch', localDir: 'tests', - remoteHost: 'https://cloudinary.netlify.app' + remoteHost: 'https://cloudinary.netlify.app', + loadingStrategy: 'lazy' }); expect(html).toEqual(`

`); diff --git a/netlify-plugin-cloudinary/tests/lib/cloudinary.test.js b/netlify-plugin-cloudinary/tests/lib/cloudinary.test.js index 98e0d80..4b0f827 100644 --- a/netlify-plugin-cloudinary/tests/lib/cloudinary.test.js +++ b/netlify-plugin-cloudinary/tests/lib/cloudinary.test.js @@ -152,7 +152,8 @@ describe('lib/util', () => { const { html } = await updateHtmlImagesToCloudinary(sourceHtml, { deliveryType: 'fetch', localDir: 'tests', - remoteHost: 'https://cloudinary.netlify.app' + remoteHost: 'https://cloudinary.netlify.app', + loadingStrategy: 'lazy' }); expect(html).toEqual(`

`); @@ -162,7 +163,8 @@ describe('lib/util', () => { const sourceHtml = '

'; const { html } = await updateHtmlImagesToCloudinary(sourceHtml, { - deliveryType: 'fetch' + deliveryType: 'fetch', + loadingStrategy: 'lazy' }); expect(html).toEqual(`

`); @@ -176,23 +178,12 @@ describe('lib/util', () => { deliveryType: 'fetch', localDir: 'tests', remoteHost: 'https://cloudinary.netlify.app', + loadingStrategy: 'lazy' }); expect(html).toEqual(`

`); }); - it('should add lazy loading to image when no option is provided', async () => { - const sourceHtml = '

'; - - const { html } = await updateHtmlImagesToCloudinary(sourceHtml, { - deliveryType: 'fetch', - localDir: 'tests', - remoteHost: 'https://cloudinary.netlify.app', - }); - - expect(html).toEqual(`

`); - }); - it('should add eager loading to image when eager option is provided for loadingStrategy', async () => { const sourceHtml = '

'; @@ -213,6 +204,7 @@ describe('lib/util', () => { deliveryType: 'upload', localDir: 'demo/.next', remoteHost: 'https://main--netlify-plugin-cloudinary.netlify.app', + loadingStrategy: 'lazy', folder: 'netlify-plugin-cloudinary', assets: mockDemo.assets }); diff --git a/netlify-plugin-cloudinary/tests/on-post-build.test.js b/netlify-plugin-cloudinary/tests/on-post-build.test.js index d5aa544..8c6e487 100644 --- a/netlify-plugin-cloudinary/tests/on-post-build.test.js +++ b/netlify-plugin-cloudinary/tests/on-post-build.test.js @@ -6,6 +6,9 @@ import { onPostBuild } from '../src/'; const mocksPath = path.join(__dirname, 'mocks/html'); const tempPath = path.join(mocksPath, 'temp'); +// Avoid illegal characters in file paths, all operating systems +const replaceRegEx = /[\W_]+/g +const replaceValue = '_' async function mkdir(directoryPath) { let dir; @@ -36,7 +39,7 @@ describe('onPostBuild', () => { process.env.CLOUDINARY_API_SECRET = 'abcd1234'; const mockFiles = (await fs.readdir(mocksPath)).filter(filePath => filePath.includes('.html')); - const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace('/', '_')); + const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue)); await mkdir(tempTestPath); await Promise.all(mockFiles.map(async file => { await fs.copyFile(path.join(mocksPath, file), path.join(tempTestPath, file)); @@ -46,7 +49,7 @@ describe('onPostBuild', () => { afterEach(async () => { process.env = ENV_ORIGINAL; - await fs.rm(path.join(tempPath, expect.getState().currentTestName.replace('/', '_')), { recursive: true, force: true }); + await fs.rm(path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue)), { recursive: true, force: true }); }); afterAll(async () => { @@ -66,7 +69,7 @@ describe('onPostBuild', () => { const deliveryType = 'fetch'; - const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace('/', '_')); + const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue)); await onPostBuild({ constants: { @@ -105,7 +108,7 @@ describe('onPostBuild', () => { const deliveryType = 'fetch'; - const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace('/', '_')); + const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue)); const maxSize = { width: 800, @@ -140,4 +143,50 @@ describe('onPostBuild', () => { }); + describe('loadingStrategy', () => { + test.each([ + {loadingStrategy: undefined, expected: 'lazy'}, + {loadingStrategy: 'lazy', expected: 'lazy'}, + {loadingStrategy: 'eager', expected: 'eager'}, + ])('should use $expected as img loading attribute when netlify.toml loadingStrategy is $loadingStrategy', async ({loadingStrategy, expected}) => { + process.env.CONTEXT = 'production'; + process.env.NETLIFY_HOST = 'https://netlify-plugin-cloudinary.netlify.app'; + + // Tests to ensure that delivery type of fetch works without API Key and Secret as it should + + delete process.env.CLOUDINARY_API_KEY; + delete process.env.CLOUDINARY_API_SECRET; + + const deliveryType = 'fetch'; + + const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue)); + + const inputs = { + deliveryType, + folder: process.env.SITE_NAME + } + + if (loadingStrategy != undefined) { + inputs['loadingStrategy'] = loadingStrategy + } + + await onPostBuild({ + constants: { + PUBLISH_DIR: tempTestPath + }, + inputs: inputs, + }); + + const files = await fs.readdir(tempTestPath); + + await Promise.all(files.map(async file => { + const data = await fs.readFile(path.join(tempTestPath, file), 'utf-8'); + const dom = new JSDOM(data); + const images = Array.from(dom.window.document.querySelectorAll('img')); + images.forEach(image => { + expect(image.getAttribute('loading')).toMatch(expected); + }) + })); + }) + }); }); From b266e6cf150768b5976cc113ee5aef4d4d820dae Mon Sep 17 00:00:00 2001 From: semantic-release-bot Date: Mon, 13 Nov 2023 18:52:57 +0000 Subject: [PATCH 2/3] chore(release): 1.15.0 [skip ci] # [1.15.0](https://github.com/colbyfayock/netlify-plugin-cloudinary/compare/v1.14.0...v1.15.0) (2023-11-13) ### Features * Get loadingStrategy value from netlify.toml inputs, default to 'lazy' if not provided ([#98](https://github.com/colbyfayock/netlify-plugin-cloudinary/issues/98)) ([8d68b05](https://github.com/colbyfayock/netlify-plugin-cloudinary/commit/8d68b05b35edaf641860822a6dad13989a582854)) --- CHANGELOG.md | 7 +++++++ netlify-plugin-cloudinary/package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cf1782..96baf9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# [1.15.0](https://github.com/colbyfayock/netlify-plugin-cloudinary/compare/v1.14.0...v1.15.0) (2023-11-13) + + +### Features + +* Get loadingStrategy value from netlify.toml inputs, default to 'lazy' if not provided ([#98](https://github.com/colbyfayock/netlify-plugin-cloudinary/issues/98)) ([8d68b05](https://github.com/colbyfayock/netlify-plugin-cloudinary/commit/8d68b05b35edaf641860822a6dad13989a582854)) + # [1.14.0](https://github.com/colbyfayock/netlify-plugin-cloudinary/compare/v1.13.1...v1.14.0) (2023-11-10) diff --git a/netlify-plugin-cloudinary/package.json b/netlify-plugin-cloudinary/package.json index 3ad32c2..5dc95cf 100644 --- a/netlify-plugin-cloudinary/package.json +++ b/netlify-plugin-cloudinary/package.json @@ -1,6 +1,6 @@ { "name": "netlify-plugin-cloudinary", - "version": "1.14.0", + "version": "1.15.0", "description": "Supercharge images on your Netlify site with Cloudinary!", "main": "dist/index.js", "scripts": { From becc1900ccef83c31ba645c3f89f3de466bc1ad3 Mon Sep 17 00:00:00 2001 From: "allcontributors[bot]" <46447321+allcontributors[bot]@users.noreply.github.com> Date: Mon, 13 Nov 2023 14:17:50 -0500 Subject: [PATCH 3/3] docs: add gshel as a contributor for code (#99) Adds @gshel as a contributor for code. This was requested by colbyfayock [in this comment](https://github.com/cloudinary-community/netlify-plugin-cloudinary/pull/98#issuecomment-1808807984) [skip ci] --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> --- .all-contributorsrc | 9 +++++++++ README.md | 1 + 2 files changed, 10 insertions(+) diff --git a/.all-contributorsrc b/.all-contributorsrc index 5ce4074..01bc24a 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -118,6 +118,15 @@ "contributions": [ "code" ] + }, + { + "login": "gshel", + "name": "Gretchen Shelby-Dormer", + "avatar_url": "https://avatars.githubusercontent.com/u/35184207?v=4", + "profile": "https://gshel.org", + "contributions": [ + "code" + ] } ], "contributorsPerLine": 7, diff --git a/README.md b/README.md index 2576200..a4d71b7 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,7 @@ npm run test Erica Pisani
Erica Pisani
💻 Matías Hernández Arellano
Matías Hernández Arellano
💻 Kieran Klukas
Kieran Klukas
💻 + Gretchen Shelby-Dormer
Gretchen Shelby-Dormer
💻