Skip to content

Commit 8d68b05

Browse files
gshelgshel
authored
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 #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
1 parent 57561c8 commit 8d68b05

File tree

6 files changed

+68
-23
lines changed

6 files changed

+68
-23
lines changed

‎netlify-plugin-cloudinary/src/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ export async function onBuild({
277277
}
278278

279279
mediaPaths.forEach(async mediaPath => {
280-
const cldAssetPath = `/${path.join(PUBLIC_ASSET_PATH, mediaPath)}`;
280+
const cldAssetPath = `/${path.posix.join(PUBLIC_ASSET_PATH, mediaPath)}`;
281281
const cldAssetUrl = `${host}${cldAssetPath}`;
282282
try {
283283
const { cloudinaryUrl: assetRedirectUrl } = await getCloudinaryUrl({
@@ -335,6 +335,7 @@ export async function onPostBuild({
335335
cname,
336336
deliveryType,
337337
folder = process.env.SITE_NAME,
338+
loadingStrategy = inputs.loadingStrategy || 'lazy',
338339
privateCdn,
339340
uploadPreset,
340341
} = inputs;
@@ -387,6 +388,7 @@ export async function onPostBuild({
387388
deliveryType,
388389
uploadPreset,
389390
folder,
391+
loadingStrategy,
390392
localDir: PUBLISH_DIR,
391393
remoteHost: host,
392394
transformations

‎netlify-plugin-cloudinary/src/lib/cloudinary.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export type Assets = {
7070

7171
type UpdateCloudinaryOptions = Omit<CloudinaryOptions, 'path'> & {
7272
assets: Assets;
73-
loadingStrategy?: "lazy"
73+
loadingStrategy: string;
7474
}
7575

7676
/**
@@ -288,7 +288,7 @@ export async function updateHtmlImagesToCloudinary(html: string, options: Update
288288
folder,
289289
localDir,
290290
remoteHost,
291-
loadingStrategy = 'lazy',
291+
loadingStrategy,
292292
transformations
293293
} = options
294294

‎netlify-plugin-cloudinary/tests/lib/cloudinary-cname.test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ describe('lib/util', () => {
4949
const { html } = await updateHtmlImagesToCloudinary(sourceHtml, {
5050
deliveryType: 'fetch',
5151
localDir: 'tests',
52-
remoteHost: 'https://cloudinary.netlify.app'
52+
remoteHost: 'https://cloudinary.netlify.app',
53+
loadingStrategy: 'lazy'
5354
});
5455

5556
expect(html).toEqual(`<html><head></head><body><p><img src=\"https://${cname}/image/fetch/f_auto,q_auto/https://cloudinary.netlify.app/images/stranger-things-dustin.jpeg\" loading=\"lazy"\></p></body></html>`);

‎netlify-plugin-cloudinary/tests/lib/cloudinary-privatecdn.test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ describe('lib/util', () => {
4848
const { html } = await updateHtmlImagesToCloudinary(sourceHtml, {
4949
deliveryType: 'fetch',
5050
localDir: 'tests',
51-
remoteHost: 'https://cloudinary.netlify.app'
51+
remoteHost: 'https://cloudinary.netlify.app',
52+
loadingStrategy: 'lazy'
5253
});
5354

5455
expect(html).toEqual(`<html><head></head><body><p><img src=\"https://${process.env.CLOUDINARY_CLOUD_NAME}-res.cloudinary.com/image/fetch/f_auto,q_auto/https://cloudinary.netlify.app/images/stranger-things-dustin.jpeg\" loading=\"lazy"\></p></body></html>`);

‎netlify-plugin-cloudinary/tests/lib/cloudinary.test.js‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ describe('lib/util', () => {
152152
const { html } = await updateHtmlImagesToCloudinary(sourceHtml, {
153153
deliveryType: 'fetch',
154154
localDir: 'tests',
155-
remoteHost: 'https://cloudinary.netlify.app'
155+
remoteHost: 'https://cloudinary.netlify.app',
156+
loadingStrategy: 'lazy'
156157
});
157158

158159
expect(html).toEqual(`<html><head></head><body><p><img src=\"https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://cloudinary.netlify.app/images/stranger-things-dustin.jpeg\" loading=\"lazy"\></p></body></html>`);
@@ -162,7 +163,8 @@ describe('lib/util', () => {
162163
const sourceHtml = '<html><head></head><body><p><img src="https://i.imgur.com/vtYmp1x.png"></p></body></html>';
163164

164165
const { html } = await updateHtmlImagesToCloudinary(sourceHtml, {
165-
deliveryType: 'fetch'
166+
deliveryType: 'fetch',
167+
loadingStrategy: 'lazy'
166168
});
167169

168170
expect(html).toEqual(`<html><head></head><body><p><img src=\"https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://i.imgur.com/vtYmp1x.png\" loading=\"lazy"\></p></body></html>`);
@@ -176,23 +178,12 @@ describe('lib/util', () => {
176178
deliveryType: 'fetch',
177179
localDir: 'tests',
178180
remoteHost: 'https://cloudinary.netlify.app',
181+
loadingStrategy: 'lazy'
179182
});
180183

181184
expect(html).toEqual(`<html><head></head><body><p><img src=\"https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://cloudinary.netlify.app/images/stranger-things-dustin.jpeg\" srcset=\"https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://cloudinary.netlify.app/images/stranger-things-dustin.jpeg 1x, https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://cloudinary.netlify.app/images/stranger-things-dustin.jpeg 2x\" loading=\"lazy"\></p></body></html>`);
182185
});
183186

184-
it('should add lazy loading to image when no option is provided', async () => {
185-
const sourceHtml = '<html><head></head><body><p><img src="https://i.imgur.com/vtYmp1x.png"></p></body></html>';
186-
187-
const { html } = await updateHtmlImagesToCloudinary(sourceHtml, {
188-
deliveryType: 'fetch',
189-
localDir: 'tests',
190-
remoteHost: 'https://cloudinary.netlify.app',
191-
});
192-
193-
expect(html).toEqual(`<html><head></head><body><p><img src=\"https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://i.imgur.com/vtYmp1x.png\" loading=\"lazy"\></p></body></html>`);
194-
});
195-
196187
it('should add eager loading to image when eager option is provided for loadingStrategy', async () => {
197188
const sourceHtml = '<html><head></head><body><p><img src="https://i.imgur.com/vtYmp1x.png"></p></body></html>';
198189

@@ -213,6 +204,7 @@ describe('lib/util', () => {
213204
deliveryType: 'upload',
214205
localDir: 'demo/.next',
215206
remoteHost: 'https://main--netlify-plugin-cloudinary.netlify.app',
207+
loadingStrategy: 'lazy',
216208
folder: 'netlify-plugin-cloudinary',
217209
assets: mockDemo.assets
218210
});

‎netlify-plugin-cloudinary/tests/on-post-build.test.js‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import { onPostBuild } from '../src/';
66

77
const mocksPath = path.join(__dirname, 'mocks/html');
88
const tempPath = path.join(mocksPath, 'temp');
9+
// Avoid illegal characters in file paths, all operating systems
10+
const replaceRegEx = /[\W_]+/g
11+
const replaceValue = '_'
912

1013
async function mkdir(directoryPath) {
1114
let dir;
@@ -36,7 +39,7 @@ describe('onPostBuild', () => {
3639
process.env.CLOUDINARY_API_SECRET = 'abcd1234';
3740

3841
const mockFiles = (await fs.readdir(mocksPath)).filter(filePath => filePath.includes('.html'));
39-
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace('/', '_'));
42+
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue));
4043
await mkdir(tempTestPath);
4144
await Promise.all(mockFiles.map(async file => {
4245
await fs.copyFile(path.join(mocksPath, file), path.join(tempTestPath, file));
@@ -46,7 +49,7 @@ describe('onPostBuild', () => {
4649
afterEach(async () => {
4750
process.env = ENV_ORIGINAL;
4851

49-
await fs.rm(path.join(tempPath, expect.getState().currentTestName.replace('/', '_')), { recursive: true, force: true });
52+
await fs.rm(path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue)), { recursive: true, force: true });
5053
});
5154

5255
afterAll(async () => {
@@ -66,7 +69,7 @@ describe('onPostBuild', () => {
6669

6770
const deliveryType = 'fetch';
6871

69-
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace('/', '_'));
72+
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue));
7073

7174
await onPostBuild({
7275
constants: {
@@ -105,7 +108,7 @@ describe('onPostBuild', () => {
105108

106109
const deliveryType = 'fetch';
107110

108-
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace('/', '_'));
111+
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue));
109112

110113
const maxSize = {
111114
width: 800,
@@ -140,4 +143,50 @@ describe('onPostBuild', () => {
140143

141144
});
142145

146+
describe('loadingStrategy', () => {
147+
test.each([
148+
{loadingStrategy: undefined, expected: 'lazy'},
149+
{loadingStrategy: 'lazy', expected: 'lazy'},
150+
{loadingStrategy: 'eager', expected: 'eager'},
151+
])('should use $expected as img loading attribute when netlify.toml loadingStrategy is $loadingStrategy', async ({loadingStrategy, expected}) => {
152+
process.env.CONTEXT = 'production';
153+
process.env.NETLIFY_HOST = 'https://netlify-plugin-cloudinary.netlify.app';
154+
155+
// Tests to ensure that delivery type of fetch works without API Key and Secret as it should
156+
157+
delete process.env.CLOUDINARY_API_KEY;
158+
delete process.env.CLOUDINARY_API_SECRET;
159+
160+
const deliveryType = 'fetch';
161+
162+
const tempTestPath = path.join(tempPath, expect.getState().currentTestName.replace(replaceRegEx, replaceValue));
163+
164+
const inputs = {
165+
deliveryType,
166+
folder: process.env.SITE_NAME
167+
}
168+
169+
if (loadingStrategy != undefined) {
170+
inputs['loadingStrategy'] = loadingStrategy
171+
}
172+
173+
await onPostBuild({
174+
constants: {
175+
PUBLISH_DIR: tempTestPath
176+
},
177+
inputs: inputs,
178+
});
179+
180+
const files = await fs.readdir(tempTestPath);
181+
182+
await Promise.all(files.map(async file => {
183+
const data = await fs.readFile(path.join(tempTestPath, file), 'utf-8');
184+
const dom = new JSDOM(data);
185+
const images = Array.from(dom.window.document.querySelectorAll('img'));
186+
images.forEach(image => {
187+
expect(image.getAttribute('loading')).toMatch(expected);
188+
})
189+
}));
190+
})
191+
});
143192
});

0 commit comments

Comments
 (0)