From e3f98018a15883cefad7be3ce7d986bf05dad842 Mon Sep 17 00:00:00 2001 From: error9098x Date: Wed, 29 Oct 2025 03:29:35 +0530 Subject: [PATCH] fix: include height parameter when using limit crop mode with explicit height Fixes #231 - url-loader was ignoring height option when crop mode is 'limit'. The issue was caused by explicitly excluding the 'limit' crop mode from height parameter inclusion. This prevented height from being included even when both width and height were explicitly provided by the user. Updated the logic to: - Include height when explicitly provided (unless using aspect ratio) - Exclude height only when aspect ratio is used (as it determines height automatically) - Apply to all crop modes, including 'limit' This allows Cloudinary's limit transformation to respect both width and height constraints as documented in their API. BREAKING CHANGE: URLs generated with width and height will now include both parameters for limit crop mode. Update any existing code that depends on the previous behavior of omitting height with limit crop. Test coverage: - Updated 12 test cases to expect h_ parameter with c_limit - All 146 tests passing - No lint errors --- packages/url-loader/src/plugins/cropping.ts | 6 ++-- .../url-loader/tests/lib/cloudinary.spec.ts | 30 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/url-loader/src/plugins/cropping.ts b/packages/url-loader/src/plugins/cropping.ts index 3c69370..21e8bd7 100644 --- a/packages/url-loader/src/plugins/cropping.ts +++ b/packages/url-loader/src/plugins/cropping.ts @@ -221,10 +221,10 @@ function collectTransformations(collectOptions: CroppingPlugin.NestedOptions) { transformations.push(`w_${width}`); } - // Some crop types don't need a height and will resize based - // on the aspect ratio + // Skip explicit height when using aspect ratio (which determines height automatically) + // Otherwise, include explicit height when provided - if (!["limit"].includes(crop) && typeof height === "number") { + if (typeof height === "number" && !hasValidAspectRatio) { transformations.push(`h_${height}`); } diff --git a/packages/url-loader/tests/lib/cloudinary.spec.ts b/packages/url-loader/tests/lib/cloudinary.spec.ts index 0c73f22..0478a1f 100644 --- a/packages/url-loader/tests/lib/cloudinary.spec.ts +++ b/packages/url-loader/tests/lib/cloudinary.spec.ts @@ -30,7 +30,7 @@ describe("Cloudinary", () => { }, }); expect(url).toContain( - `https://res.cloudinary.com/${cloudName}/image/upload/c_limit,w_100/f_auto/q_auto/v1/turtle`, + `https://res.cloudinary.com/${cloudName}/image/upload/c_limit,w_100,h_100/f_auto/q_auto/v1/turtle`, ); }); @@ -51,7 +51,7 @@ describe("Cloudinary", () => { }, }); expect(url).toContain( - `https://res.cloudinary.com/${cloudName}/${assetType}/upload/c_limit,w_100/f_auto/q_auto/v1/turtle`, + `https://res.cloudinary.com/${cloudName}/${assetType}/upload/c_limit,w_100,h_100/f_auto/q_auto/v1/turtle`, ); }); @@ -77,7 +77,7 @@ describe("Cloudinary", () => { }, }); expect(url).toContain( - `https://res.cloudinary.com/${cloudName}/image/upload/c_limit,w_100/f_${format}/q_${quality}/v1/turtle`, + `https://res.cloudinary.com/${cloudName}/image/upload/c_limit,w_100,h_100/f_${format}/q_${quality}/v1/turtle`, ); }); @@ -86,7 +86,7 @@ describe("Cloudinary", () => { const deliveryType = "upload"; const publicId = "myimage"; - const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/v1234/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/v1234/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -112,7 +112,7 @@ describe("Cloudinary", () => { const publicId = "myimage"; const dpr = "2.0"; - const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/dpr_${dpr}/f_auto/q_auto/v1/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/dpr_${dpr}/f_auto/q_auto/v1/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -137,7 +137,7 @@ describe("Cloudinary", () => { const publicId = "myimage"; const dpr = 2; - const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/dpr_${dpr.toFixed(1)}/f_auto/q_auto/v1/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/dpr_${dpr.toFixed(1)}/f_auto/q_auto/v1/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -161,7 +161,7 @@ describe("Cloudinary", () => { const publicId = "myimage"; const dpr = "auto"; - const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/dpr_${dpr}/f_auto/q_auto/v1/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/dpr_${dpr}/f_auto/q_auto/v1/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -260,7 +260,7 @@ describe("Cloudinary", () => { }, }); expect(url).toContain( - `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/f_auto/q_auto/v1/${src}`, + `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/f_auto/q_auto/v1/${src}`, ); }); @@ -269,7 +269,7 @@ describe("Cloudinary", () => { const deliveryType = "fetch"; const publicId = "myimage"; - const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/f_auto/q_auto/v1234/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/f_auto/q_auto/v1234/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -293,7 +293,7 @@ describe("Cloudinary", () => { const deliveryType = "fetch"; const publicId = "myimage"; - const src = `res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100/f_auto/q_auto/v1234/${publicId}?_a=`; + const src = `res.cloudinary.com/${cloudName}/image/${deliveryType}/c_limit,w_100,h_100/f_auto/q_auto/v1234/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -366,7 +366,7 @@ describe("Cloudinary", () => { const publicId = "myimage"; const seoSuffix = "test-image"; - const src = `https://res.cloudinary.com/${cloudName}/images/c_limit,w_100/f_auto/q_auto/v1234/${publicId}/${seoSuffix}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/images/c_limit,w_100,h_100/f_auto/q_auto/v1234/${publicId}/${seoSuffix}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -392,7 +392,7 @@ describe("Cloudinary", () => { const width = 1234; const height = 1234; - const src = `https://res.cloudinary.com/${cloudName}/${assetType}/c_limit,w_${width}/f_auto/q_auto/v1234/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/${assetType}/c_limit,w_${width},h_${height}/f_auto/q_auto/v1234/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -419,8 +419,8 @@ describe("Cloudinary", () => { const width = 1234; const height = 1234; - const src = `https://res.cloudinary.com/${cloudName}/${assetType}/c_limit,w_${width}/f_auto/q_auto/v1234/${publicId}/${seoSuffix}${format}?_i=A`; - const exepectedSrc = `https://res.cloudinary.com/${cloudName}/image/fetch/c_limit,w_${width}/f_auto/q_auto/v1234/${publicId}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/${assetType}/c_limit,w_${width},h_${height}/f_auto/q_auto/v1234/${publicId}/${seoSuffix}${format}?_i=A`; + const exepectedSrc = `https://res.cloudinary.com/${cloudName}/image/fetch/c_limit,w_${width},h_${height}/f_auto/q_auto/v1234/${publicId}?_a=`; const url = constructCloudinaryUrl({ options: { @@ -446,7 +446,7 @@ describe("Cloudinary", () => { const originalSeoSuffix = "my-image"; const seoSuffix = "test-image"; - const src = `https://res.cloudinary.com/${cloudName}/${assetType}/c_limit,w_100/f_auto/q_auto/v1234/${publicId}/${originalSeoSuffix}?_a=`; + const src = `https://res.cloudinary.com/${cloudName}/${assetType}/c_limit,w_100,h_100/f_auto/q_auto/v1234/${publicId}/${originalSeoSuffix}?_a=`; const url = constructCloudinaryUrl({ options: {