Skip to content

fix(cli-upload): pin image-size to ~1.0.2 to keep Node 14 support#2301

Open
rishigupta1599 wants to merge 1 commit into
masterfrom
fix/cli-upload-image-size-node14
Open

fix(cli-upload): pin image-size to ~1.0.2 to keep Node 14 support#2301
rishigupta1599 wants to merge 1 commit into
masterfrom
fix/cli-upload-image-size-node14

Conversation

@rishigupta1599

Copy link
Copy Markdown
Contributor

Problem

@percy/cli declares engines: { node: ">=14" }, but @percy/cli-upload declared its image-size dependency as the loose range "^1.0.0".

image-size raised its own engine requirement to node >=16 starting at v1.1.1 (1.0.2 = >=14, 1.1.1 / 1.2.0 / 1.2.1 = >=16). Because ^1.0.0 resolves to the latest matching version, any consumer that regenerates its lockfile against @percy/cli now pulls image-size@1.2.1 (node>=16) — silently violating @percy/cli's own engines: node>=14.

This isn't a code change in any recent CLI release — the ^1.0.0 range has been here since @percy/cli-upload was first added. It only surfaces now because (a) image-size published a node-bumping release and (b) downstream lockfiles get regenerated (e.g. when SDKs bump @percy/cli).

Observed impact

Downstream Percy SDKs that still test Node 14 (percy-puppeteer, percy-selenium-js, percy-testcafe, percy-nightmare) fail their Node 14 CI legs on install, because the transitive image-size@1.2.1 requires node>=16 even though @percy/cli claims >=14.

Fix

Constrain the range to ~1.0.2 (>=1.0.2 <1.1.0) — the last image-size line that supports node>=14 — so @percy/cli honors its own engines field again.

  • The CLI monorepo's own lockfile already resolved image-size@1.0.2; this just keeps every downstream consumer pinned to that line too.
  • image-size@1.0.x is API-compatible with cli-upload's usage (reading image dimensions), so no functional change.

Alternative considered

Officially dropping Node 14 (bump engines to >=16 across the CLI + SDK matrices). That's a larger, breaking decision; this PR is the minimal fix that restores the currently declared support contract. Happy to go the other direction if maintainers prefer to drop Node 14.

🤖 Generated with Claude Code

@percy/cli-upload declared image-size as "^1.0.0", a loose range. image-size
raised its engine requirement to node>=16 starting at v1.1.1, so any consumer
regenerating its lockfile against @percy/cli now resolves image-size@1.2.1
(node>=16) — silently breaking @percy/cli's own declared `engines: node>=14`.

This surfaces in downstream SDKs (percy-puppeteer, percy-selenium-js,
percy-testcafe, percy-nightmare): their Node 14 CI legs fail on install because
the transitive image-size requires node>=16, even though @percy/cli claims >=14.

Constrain the range to ~1.0.2 (>=1.0.2 <1.1.0) — the last image-size line that
supports node>=14 — so @percy/cli honors its own engines field again. The CLI's
own lockfile already resolved 1.0.2; this just keeps every downstream consumer
on that line. image-size 1.0.x is API-compatible with cli-upload's usage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rishigupta1599 rishigupta1599 requested a review from a team as a code owner June 17, 2026 11:48
rishigupta1599 added a commit to percy/percy-puppeteer that referenced this pull request Jun 17, 2026
@percy/cli (>=14) transitively pulls image-size via @percy/cli-upload's loose
"^1.0.0" range, which now resolves to image-size@1.2.1 (requires node>=16),
breaking this SDK's Node 14 CI leg on install.

Force image-size back to the node>=14-compatible 1.0.x line. Temporary
workaround pending the upstream CLI fix (percy/cli#2301); remove once this SDK
bumps to the patched @percy/cli release.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@rishigupta1599 rishigupta1599 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review (automated) — 1 inline finding(s). Full report in the PR comment below. Verdict: Passed.

"@percy/cli-command": "1.32.0",
"fast-glob": "^3.2.11",
"image-size": "^1.0.0"
"image-size": "~1.0.2"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low] Range works, but consider ~1.0.0

~1.0.2 (>=1.0.2 <1.1.0) correctly excludes image-size@1.1.0+ (which drops Node 14), so the fix is valid. It currently resolves to exactly 1.0.2. If the intent is just "any safe 1.0.x, exclude the breaking 1.1.0 minor", ~1.0.0 expresses that slightly more idiomatically and admits future 1.0.x patches.

Suggestion: Optional — keep ~1.0.2 if a >=1.0.2 floor is intended; otherwise ~1.0.0 is marginally more conventional.

Reviewer: stack:code-reviewer

@rishigupta1599

Copy link
Copy Markdown
Contributor Author

Claude Code PR Review

PR: #2301Head: 00427b8Reviewers: stack:code-reviewer

Summary

Pins the image-size dependency in @percy/cli-upload from ^1.0.0 (>=1.0.0 <2.0.0) to ~1.0.2 (>=1.0.2 <1.1.0) to keep Node 14 compatibility, since image-size@1.1.0+ raises its Node engine requirement to >=16/>=18. The yarn.lock entry key is updated to match.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass Dependency-range change only.
High Security Authentication/authorization checks present N/A No auth code touched.
High Security Input validation and sanitization N/A No input handling touched.
High Security No IDOR — resource ownership validated N/A Not applicable.
High Security No SQL injection (parameterized queries) N/A No DB code.
High Correctness Logic is correct, handles edge cases Pass ~1.0.2 correctly excludes 1.1.0+, achieving the Node 14 goal.
High Correctness Error handling is explicit, no swallowed exceptions N/A No logic changes.
High Correctness No race conditions or concurrency issues N/A Not applicable.
Medium Testing New code has corresponding tests N/A Dependency pin; existing Node 14 CI matrix covers it.
Medium Testing Error paths and edge cases tested N/A Not applicable.
Medium Testing Existing tests still pass (no regressions) Pass Resolved version unchanged (1.0.2); no behavioral change.
Medium Performance No N+1 queries or unbounded data fetching N/A Not applicable.
Medium Performance Long-running tasks use background jobs N/A Not applicable.
Medium Quality Follows existing codebase patterns Pass package.json + yarn.lock key updated consistently.
Medium Quality Changes are focused (single concern) Pass Single dependency pin.
Low Quality Meaningful names, no dead code Pass N/A.
Low Quality Comments explain why, not what Pass Commit message states the Node 14 rationale.
Low Quality No unnecessary dependencies added Pass No new dependency; tightens an existing range.

Findings

No blocking findings. One optional style note below.

  • File: packages/cli-upload/package.json:38
  • Severity: Low
  • Reviewer: stack:code-reviewer (severity corrected by orchestrator)
  • Issue: ~1.0.2 (>=1.0.2 <1.1.0) currently resolves to exactly 1.0.2, since no later 1.0.x patch exists. It works correctly. ~1.0.0 would express the same "any safe 1.0.x, exclude the breaking 1.1.0 minor" intent slightly more idiomatically and admit future 1.0.x patches.
  • Suggestion: Optional — either form is correct and both exclude 1.1.0+. Keep ~1.0.2 if a >=1.0.2 floor is intended; otherwise ~1.0.0 is marginally more conventional.

Note on the reviewer's CRITICAL (yarn.lock "manual edit")

The sub-reviewer flagged the lockfile change as a manual edit that doesn't match package.json. This was verified as a false positive and is not counted against the verdict:

  • The lockfile key was updated from image-size@^1.0.0: to image-size@~1.0.2:, which matches package.json exactly.
  • The resolved version was already 1.0.2 (satisfies both the old and new range), so yarn install legitimately produces only a key rename with unchanged version/resolved/integrity. An unchanged integrity hash is the expected output when the resolved version doesn't change — not evidence of a hand edit.
  • There is only one image-size entry in the lockfile; nothing else needed collapsing.

The reviewer's MEDIUM (no CI matrix change) is informational only — the existing Node 14 CI matrix already exercises cli-upload.


Verdict: PASS — focused, correct dependency pin that achieves the stated Node 14 compatibility goal; the lockfile is consistent and the sole CRITICAL was a verified false positive.

rishigupta1599 added a commit to percy/percy-selenium-js that referenced this pull request Jun 17, 2026
@percy/cli (>=14) transitively pulls image-size via @percy/cli-upload's loose
"^1.0.0" range, which now resolves to image-size@1.2.1 (requires node>=16),
breaking this SDK's Node 14 CI leg on install.

Force image-size back to the node>=14-compatible 1.0.x line. Temporary
workaround pending the upstream CLI fix (percy/cli#2301); remove once this SDK
bumps to the patched @percy/cli release.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rishigupta1599 added a commit to percy/percy-testcafe that referenced this pull request Jun 17, 2026
@percy/cli (>=14) transitively pulls image-size via @percy/cli-upload's loose
"^1.0.0" range, which now resolves to image-size@1.2.1 (requires node>=16),
breaking this SDK's Node 14 CI leg on install.

Force image-size back to the node>=14-compatible 1.0.x line. Temporary
workaround pending the upstream CLI fix (percy/cli#2301); remove once this SDK
bumps to the patched @percy/cli release.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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