fix(cropping): respect explicit height parameter with limit crop mode

[error9098x]

Description

This PR fixes a critical bug in the url-loader's cropping plugin where the height parameter was being ignored when using the limit crop mode, even when both width and height were explicitly provided.

The Problem

When users provided both width and height with the default limit crop mode:

const url = constructCloudinaryUrl({
  options: {
    src: 'my-public-id',
    width: 800,
    height: 600
  },
  config: { cloud: { cloudName: 'my-cloud' } }
});

Expected: c_limit,w_800,h_600
Actual (Bug): c_limit,w_800 ❌

This meant the height constraint was never applied, only width was enforced.

The Solution

Updated the height inclusion logic from checking if crop is "limit" to checking if an aspect ratio is being used:

• Include height when explicitly provided AND no aspect ratio is set
• Exclude height only when using aspect ratio (which calculates height automatically)
• Apply to ALL crop modes, including "limit"

Flow Diagram

flowchart TD
    A["User provides width & height"] --> B{"Is aspect ratio used?"}
    B -- Yes --> C["Exclude height<br>(AR determines height)"]
    B -- No --> D["Include height<br>(h_ parameter added)"]
    C --> E["Generated URL:<br>c_MODE,w_X,ar_Y"]
    D --> F["Generated URL:<br>c_MODE,w_X,h_Y"]
    style C fill:#e1f5ff,color:#000000
    style D fill:#c8e6c9,color:#000000
    style E fill:#e1f5ff,color:#000000
    style F fill:#c8e6c9,color:#000000

Loading

Issue Ticket Number

Fixes #231

Type of change

• 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)
• Fix or improve the documentation
• This change requires a documentation update

---

Changes Made

1. Core Bug Fix

File: packages/url-loader/src/plugins/cropping.ts (Lines 224-229)

- // 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}`);
  }

2. Test Updates

File: packages/url-loader/tests/lib/cloudinary.spec.ts

Updated 12 test cases to expect the height parameter in URLs when both width and height are provided:

• Line 33: c_limit,w_100,h_100 ✓
• Line 54: c_limit,w_100,h_100 ✓
• Line 80: c_limit,w_100,h_100 ✓
• Line 89: c_limit,w_100,h_100 ✓
• Line 115, 140, 164: DPR tests
• Line 263, 272, 296: Delivery type tests
• Line 369, 395, 449: SEO tests

---

Test Results

✅ All Tests Passing

Test Files  24 passed (24)
Tests       146 passed (146)
Duration    2.19s

Specific Test File Results

Test File	Status	Count
cloudinary.spec.ts	✅ PASS	34 tests
cropping.spec.ts	✅ PASS	23 tests
fill-background.spec.ts	✅ PASS	5 tests
overlays.spec.ts	✅ PASS	16 tests
remove.spec.ts	✅ PASS	6 tests
Other plugin tests	✅ PASS	62 tests

Build & Lint Verification

graph LR
    A["Build"] -->|✅ Success| B["Type Check"]
    B -->|✅ Clean| C["Lint"]
    C -->|✅ No Errors| D["Tests"]
    D -->|✅ All Pass| E["Ready to Merge"]
    style A fill:#c8e6c9,color:#000000
    style B fill:#c8e6c9,color:#000000
    style C fill:#c8e6c9,color:#000000
    style D fill:#c8e6c9,color:#000000
    style E fill:#c8e6c9,color:#000000

Loading

---

Before & After Comparison

Before (Bug)

// Input
{ width: 800, height: 600, crop: 'limit' }

// Output (WRONG)
https://res.cloudinary.com/mycloud/image/upload/c_limit,w_800/...
                                          ↑ height missing!

After (Fixed)

// Input
{ width: 800, height: 600, crop: 'limit' }

// Output (CORRECT)
https://res.cloudinary.com/mycloud/image/upload/c_limit,w_800,h_600/...
                                          ↑ both constraints now applied!

Other Crop Modes (Unchanged, Correct Behavior)

// With aspect ratio - height is correctly excluded
{ width: 960, aspectRatio: '16:9', crop: 'fill' }
// Output: c_fill,ar_16:9,w_960  ✓

// Without aspect ratio - height is included
{ width: 960, height: 540, crop: 'fill' }
// Output: c_fill,w_960,h_540,g_auto  ✓

---

Impact Analysis

Breaking Change Notice

Version Impact: 6.3.0 (or 6.2.1 depending on release strategy)

URLs generated with width AND height will now include both parameters:

- c_limit,w_800      ← OLD (incomplete)
+ c_limit,w_800,h_600 ← NEW (correct)

Migration Required: Any code relying on the old behavior of omitting height should be reviewed.

Affected Users

Users who:

• ✅ Explicitly provided both width and height with limit crop (now works correctly)
• ❌ Relied on height being ignored (edge case, likely unintended)

---

Root Cause Analysis

flowchart TD
    A["Bug Report #231"] --> B["User provided width & height"]
    B --> C@{ label: "Default crop mode: 'limit'" }
    C --> D["Height parameter missing in URL"]
    E["Root Cause Analysis"] --> F["Line 227 in cropping.ts"]
    F --> G@{ label: "Logic: exclude height if crop IS 'limit'" }
    G --> H["❌ Wrong assumption:<br>ALL crop modes should accept height"]
    I["Fix Applied"] --> J["Changed logic:<br>exclude only when aspect ratio used"]
    J --> K@{ label: "✅ Now correct:<br>height included for 'limit'" }
    C@{ shape: rect}
    G@{ shape: rect}
    K@{ shape: rect}
    style A fill:#ffcdd2,color:#000000
    style D fill:#ffcdd2,stroke:none,color:#000000
    style H fill:#ffcdd2,stroke:none,color:#000000
    style K fill:#c8e6c9,color:#000000,stroke:none

Loading

---

Code Review Checklist

• Bug fix addresses the root cause
• Logic is sound and follows Cloudinary API docs
• All existing tests pass
• New test cases added/updated
• No regression detected
• Code is well-commented
• Performance impact: none
• TypeScript types correct
• Backward compatibility: breaking (expected for bug fix)

---

Checklist

• I have followed the contributing guidelines of this project as mentioned in CONTRIBUTING.md
• I have created an issue ticket for this PR
• I have checked to ensure there aren't other open Pull Requests for the same update/change
• I have performed a self-review of my own code
• I have run tests locally to ensure they all pass
• I have commented my code, particularly in hard-to-understand areas
• This change requires a documentation update (note breaking change in BREAKING_CHANGES section)

---

Additional Context

GitHub Issue Reference

Issue #231: url-loader ignores height option when crop mode is "limit"

Commit Information

• Commit Hash: e3f9801
• Author: error9098x
• Date: Wed Oct 29 03:29:35 2025 +0530

Documentation Updates Needed

• Update CHANGELOG.md with breaking change notice
• Update README with example of limit crop with height
• Document aspect ratio behavior vs explicit dimensions

---

Dependencies

No new dependencies added - this is a logic fix only.

Package Versions Used:

• @cloudinary/url-gen: 1.15.0
• TypeScript: 5.5.2
• Vitest: 2.0.5

[error9098x]

@eportis-cloudinary Would appreciate your feedback on this approach to fixing #231!

[eportis-cloudinary]

Hi @error9098x ! Thank you for this thurough analysis and PR. On first glance I really like the approach, and this would be a breaking change that we are not going to be able to merge into the current major version. I am currently cleaning up Hacktoberfest issues and because this affects such a common, fundamental case will not be able to give this the consideration it needs over the next couple of weeks. I hope to give it a thorough review in early December. I will be in touch then. Again: thank you!