Skip to content

Fix infinite recursion and add configurable minScale in image-crop#286

Open
NicholasJAlexander wants to merge 1 commit intoshadcnblocks:mainfrom
NicholasJAlexander:fix/image-crop-infinite-recursion
Open

Fix infinite recursion and add configurable minScale in image-crop#286
NicholasJAlexander wants to merge 1 commit intoshadcnblocks:mainfrom
NicholasJAlexander:fix/image-crop-infinite-recursion

Conversation

@NicholasJAlexander
Copy link

@NicholasJAlexander NicholasJAlexander commented Oct 22, 2025

Summary

Fixed infinite recursion bug in getCroppedPngImage function and added configurable minimum scale threshold.

Problem

The getCroppedPngImage function had a bug causing infinite recursion when images exceeded maxImageSize:

  • The scaleFactor parameter was accepted but never applied to canvas dimensions
  • Canvas size remained constant across recursive calls, so blob size never changed
  • When blob.size > maxImageSize, the function recursed infinitely with no way to terminate
  • No configurable minimum threshold prevented users from scaling below 10%

Solution

  1. Apply scaleFactor to canvas dimensions - Actually downscale the image by multiplying canvas.width and canvas.height by scaleFactor
  2. Add configurable minScale parameter - Default 0.1 (10%), but users can now set it lower (e.g., 0.01 for 1%) if needed
  3. Enable image smoothing - Changed imageSmoothingEnabled from false to true with 'high' quality for better downscaling
  4. Add minScale to component props - Exposed minScale in ImageCropProps for user configuration

Changes

  • packages/image-crop/index.tsx (lines 52-110, 141-149, 151-160, 199-213)
    • Added minScale: number = 0.1 parameter to getCroppedPngImage function
    • Apply scaleFactor: canvas.width = Math.max(1, Math.floor(pixelCrop.width * scaleFactor))
    • Apply scaleFactor: canvas.height = Math.max(1, Math.floor(pixelCrop.height * scaleFactor))
    • Enable smoothing: ctx.imageSmoothingEnabled = true with ctx.imageSmoothingQuality = "high"
    • Added minScale?: number to ImageCropProps type
    • Pass minScale parameter through component to function call

Usage Example

// Default behavior (stops at 10% scale)
<ImageCrop file={file} maxImageSize={1024 * 1024} onCrop={handleCrop}>
  <ImageCropContent />
</ImageCrop>

// Custom minimum scale (can go down to 1%)
<ImageCrop 
  file={file} 
  maxImageSize={1024 * 1024}
  minScale={0.01}
  onCrop={handleCrop}
>
  <ImageCropContent />
</ImageCrop>

Testing

  • Verified the function now actually downscales images on each recursive call
  • Confirmed recursion terminates when scale reaches minimum threshold
  • Backward compatible - defaults to 0.1 if not specified

Summary by CodeRabbit

  • New Features

    • Added minScale parameter to control minimum downscaling during image cropping.
  • Improvements

    • Enhanced image quality during downscaling with improved smoothing algorithms.
    • Strengthened validation to prevent exceeding scale limits with clearer error messaging.

Fixed critical bug in getCroppedPngImage that caused infinite recursion
when images exceeded maxImageSize. The scaleFactor parameter was never
applied to canvas dimensions, resulting in identical output size on each
recursive call.

Changes:
- Apply scaleFactor to canvas width/height to actually downscale images
- Add configurable minScale parameter (default: 0.1) to prevent infinite loops
- Enable imageSmoothingEnabled with 'high' quality for better downscaling
- Add minScale prop to ImageCrop component for user configuration

Users can now set minScale lower than 0.1 (e.g., 0.01 for 1%) if needed
for stricter size requirements.
@vercel
Copy link

vercel bot commented Oct 22, 2025

@NicholasJAlexander is attempting to deploy a commit to the Haste Ventures Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

📝 Walkthrough

Walkthrough

The PR introduces a minScale parameter to enforce a minimum downscale limit during image cropping. The parameter is threaded through the component's public interface with a default value of 0.1, includes validation to prevent excessive downscaling, and enables image smoothing for improved output quality.

Changes

Cohort / File(s) Summary
Image crop scaling and validation
packages/image-crop/index.tsx
Added minScale parameter (default 0.1) to getCroppedPngImage function with validation to prevent scaleFactor from dropping below the limit; throws error when exceeded. Extended ImageCropProps interface to expose minScale as optional public prop. Threaded minScale through applyCrop call path. Enabled image smoothing and adjusted canvas dimensions to use scaled-down size (floor-based calculation).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Fix infinite recursion and add configurable minScale in image-crop" accurately reflects the main changes in the changeset. It clearly identifies the two primary objectives: fixing the infinite recursion bug in getCroppedPngImage and introducing a configurable minimum scale parameter. The title is concise, specific, and avoids vague terminology, making it easy for teammates to understand the core purpose of the change by scanning through commit history.
Description Check ✅ Passed The PR description provides comprehensive information about the changes, including a clear Summary section, detailed Problem and Solution sections explaining the bug and fix, specific file changes with line numbers, a practical usage example, and testing verification. While the description doesn't strictly follow the template structure (notably missing the "Related Issues" section and checklist items), it covers the essential information with more depth than the template requires. The content is well-organized, substantive, and allows reviewers to understand the context and rationale for the changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 518141e and ba7682f.

📒 Files selected for processing (1)
  • packages/image-crop/index.tsx (5 hunks)
🔇 Additional comments (4)
packages/image-crop/index.tsx (4)

52-64: Good recursion guard implementation.

The minScale parameter with a default of 0.1 (10%) and the early guard check effectively prevent infinite recursion. The error message provides clear guidance to users.


76-82: Core fix correctly implemented.

Applying scaleFactor to canvas dimensions is the key fix that resolves the infinite recursion bug. The use of Math.max(1, Math.floor(...)) prevents zero-sized canvases, and enabling high-quality image smoothing improves the output during downscaling.


144-144: Good API design.

The minScale parameter is correctly exposed as an optional prop with a sensible default, allowing users to customize the minimum scale threshold when needed.


154-154: Proper parameter threading.

The minScale parameter is correctly threaded from the component props through to the getCroppedPngImage function call, maintaining the default value of 0.1 throughout the chain.

Also applies to: 208-209


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant