Skip to content

fix: pin @icons-pack/react-simple-icons to ~13.11.1 to avoid EBADENGINE warning - #40284

Closed
HassanAlsheikh wants to merge 1 commit into
NousResearch:mainfrom
HassanAlsheikh:fix/icons-pack-node-engine-warning
Closed

fix: pin @icons-pack/react-simple-icons to ~13.11.1 to avoid EBADENGINE warning#40284
HassanAlsheikh wants to merge 1 commit into
NousResearch:mainfrom
HassanAlsheikh:fix/icons-pack-node-engine-warning

Conversation

@HassanAlsheikh

Copy link
Copy Markdown

Pin @icons-pack/react-simple-icons from ^13.13.0 to ~13.11.1. Version 13.11.2+ requires Node >=24, but the desktop app accepts Node 22.12+. 13.11.1 is the last release without the restriction — functionally identical, zero warnings on npm install.

@HassanAlsheikh
HassanAlsheikh requested a review from a team June 6, 2026 03:46

@alpindiay alpindiay left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Version pin for @icons-pack/react-simple-icons: ^13.13.0~13.11.1. The 13.13.0 release added restrictive engine requirements ("node": ">=24", "pnpm": ">=10") that broke compatibility with the project's Node.js version. Downgrading to 13.11.1 with a tilde constraint (~) allows only patch updates within 13.11.x, preventing future breaking engine requirements from auto-installing.

The package-lock.json changes show the new resolved version and added "peer": true markers on esbuild platform packages — those are standard lockfile regeneration artifacts from the npm/pnpm install.

No code changes, no tests needed. Straightforward dependency fix.

@alt-glitch alt-glitch added type/bug Something isn't working dependencies Pull requests that update a dependency file P3 Low — cosmetic, nice to have labels Jun 6, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ Approved

EBADENGINE is real and the fix is correct. Verified: main pins @icons-pack/react-simple-icons ^13.13.0; npm view @icons-pack/react-simple-icons@13.13.0 engines returns { node: '>=24', pnpm: '>=10' }, which fails on Node <24. Version 13.11.1 has no engines field, so the mismatch disappears.

~13.11.1 is the right constraint — locks to the 13.11.x line (below the 13.12+ releases that added node>=24) while still allowing patch bumps. Lockfile updated consistently under apps/desktop.

Reviewed by Hermes Agent

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code Review

PR #40284 — fix: pin @icons-pack/react-simple-icons to ~13.11.1 to avoid EBADENGINE warning
Verdict: ✅ Approve

Summary

Pins @icons-pack/react-simple-icons from ^13.13.0~13.11.1 in apps/desktop/package.json and updates package-lock.json accordingly.

Root Cause

react-simple-icons 13.11.2+ added "engines": { "node": ">=24" }. The desktop app targets node ^20.19.0 || >=22.12.0, so npm raises EBADENGINE on every install for 13.12.x / 13.13.x. Version 13.11.1 is functionally identical (same icon set) and carries no engine restriction.

Diff Analysis

  • ~13.11.1 correctly allows only patch-level bumps within 13.11.x — safe from accidental upcap to a Node-24-only release ✅
  • Lock file moves the resolved entry from root node_modules/@icons-pack/... to apps/desktop/node_modules/@icons-pack/... (workspace-scoped install), clean ✅
  • The removed 13.13.0 block had "engines": { "node": ">=24", "pnpm": ">=10" } — exactly the source of the EBADENGINE warning ✅

Checks

  • Main branch still has ^13.13.0; this pin is the correct fix ✅
  • No API surface change between 13.11.1 and 13.13.0 for the icons used ✅
  • Lock file integrity entries present and correct ✅

Minor notes

If the project ever requires icons added in 13.12+, a Node 24 engine bump will need to accompany the unpin. Worth a comment in the lock file or a follow-up issue — but that is out of scope here.


Reviewed by Hermes Agent

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Correct fix. @icons-pack/react-simple-icons@13.13.0 added an engines: { node: ">=24", pnpm: ">=10" } gate that fires as EBADENGINE under the project's supported Node 22.12+ range. Downgrading to 13.11.1 (the last engine-clean release) is the right call, and the package-lock.json is properly updated with a workspace-level override in apps/desktop/node_modules/@icons-pack/react-simple-icons.

One minor concern: ~13.11.1 allows npm to resolve to any future 13.11.x patch. Per the PR description, 13.11.2+ also requires Node >=24, so a future npm install or lockfile regeneration could quietly pull in 13.11.2 and re-introduce the warning. Consider using an exact pin ("13.11.1") or "=13.11.1" instead of the tilde range, since the constraint is truly 'this exact version or nothing newer until 13.12.0.' The lock file is the safety net today, but the spec itself should be defensive.

Otherwise the change is clean, minimal, and correctly scoped. ✅

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the dependency compatibility fix. This is already implemented on current main by a stronger exact pin, so this automated hermes-sweeper review is closing the redundant PR.

  • 769f307042d22be2c092249c2d8d78f85fea8e37 (fix(npm): lock react-simple-icons to 13.11.1) changed apps/desktop/package.json and package-lock.json.
  • Current apps/desktop/package.json:64 uses "@icons-pack/react-simple-icons": "=13.11.1", which also addresses the review discussion noting that ~13.11.1 could permit a later incompatible patch release.
  • Current package-lock.json:77 and package-lock.json:2781-2788 declare and resolve version 13.11.1.
  • The implementing commit is included in release v2026.6.19.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants