Skip to content

fix(desktop): pin @icons-pack/react-simple-icons below its node>=24 releases - #44197

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

fix(desktop): pin @icons-pack/react-simple-icons below its node>=24 releases#44197
xxxigm wants to merge 1 commit into
NousResearch:mainfrom
xxxigm:fix/desktop-icons-pack-node-engine

Conversation

@xxxigm

@xxxigm xxxigm commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

A WSL user (Node v22.22.3) reported a noisy desktop launch:

npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@icons-pack/react-simple-icons@13.13.0',
npm warn EBADENGINE   required: { node: '>=24', pnpm: '>=10' },
npm warn EBADENGINE   current: { node: 'v22.22.3', npm: '10.9.8' }
npm warn EBADENGINE }

apps/desktop/package.json declares it supports Node ^20.19.0 || >=22.12.0, but @icons-pack/react-simple-icons bumped its own engines.node to >=24 starting at 13.11.2 (13.11.1 and earlier have no Node floor). With the old ^13.13.0 range, every install on the project's supported Node 20/22 range prints the EBADENGINE warning — and would hard-fail under engine-strict / npm ci --engine-strict.

Fix

Cap the range at <13.11.2 (">=13.0.0 <13.11.2"), which resolves to 13.11.1 — the last release before the Node-24 floor. The package works fine on Node 22; only its declared engine was over-tightened.

Verified every brand glyph the app imports (platform-icon.tsx: Telegram, Discord, Matrix, Signal, WhatsApp, Apple, Gmail, HomeAssistant, Mattermost, QQ, WeChat, Bilibili) is present in 13.11.1 — icon-identical, no behavior change.

Lockfile diff is minimal: the resolved version moves to 13.11.1 and the old 13.13.0 entry (with engines.node: ">=24") is dropped; no other packages churn.

Test plan

  • Confirmed via the npm registry that 13.11.2+ carry engines.node: ">=24" and 13.11.1 does not.
  • Confirmed all 12 imported Si* icon modules exist in @icons-pack/react-simple-icons@13.11.1.
  • package-lock.json regenerated with npm install --package-lock-only; diff limited to this one dependency.

Note: this clears the EBADENGINE warning from the report. The reporter's separate "can't click the top two left-sidebar options" symptom looks WSL/WSLg-display-specific and is tracked separately.

…eleases

apps/desktop declares engines.node "^20.19.0 || >=22.12.0", but
@icons-pack/react-simple-icons bumped its own engines to node >=24 starting at
13.11.2. With the old "^13.13.0" range, every install on the supported Node 20/22
range emits a noisy `npm warn EBADENGINE Unsupported engine` line (and would hard
-fail under engine-strict), even though the package itself works fine on Node 22.

Cap the range at "<13.11.2" (resolves to 13.11.1, the last release with no Node
floor). All brand glyphs the app imports — Telegram, Discord, Matrix, Signal,
WhatsApp, Apple, Gmail, HomeAssistant, Mattermost, QQ, WeChat, Bilibili — are
present in 13.11.1, so this is icon-identical.
@xxxigm
xxxigm requested a review from a team June 11, 2026 10:23
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have area/config Config system, migrations, profiles duplicate This issue or pull request already exists labels Jun 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #40284 — same fix: cap @icons-pack/react-simple-icons below 13.11.2 (resolves to 13.11.1) to clear the EBADENGINE warning, since 13.11.2+ require node>=24 while the desktop app supports Node 20/22. Same files (apps/desktop/package.json, package-lock.json).

@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.

Verified — correct, minimal, and internally consistent. Approving.

Premise confirmed (via npm view)

  • @icons-pack/react-simple-icons 13.11.2, 13.12.0, 13.13.0 all declare engines: { node: ">=24" }; 13.11.1 and earlier do not.
  • Desktop app supports node: ^20.19.0 || >=22.12.0 (repo root >=20.0.0), so ^13.13.0 resolves to a node-24-only release — incompatible with the support floor. The pin >=13.0.0 <13.11.2 correctly excludes every node-24 release.

Pin/lockfile consistency ✅

apps/desktop/package.json and the lockfile root both move to >=13.0.0 <13.11.2; a nested entry resolved at 13.11.1 is added and the old hoisted 13.13.0 removed. semver confirms 13.11.1 satisfies the range and 13.13.0 does not. The 13.11.1 entry correctly drops the engines block.

Cross-PR conflict with #44024 — none

#44024 (approved, edits root package-lock.json at the @types/node region ~line 8418+ and the 19659–20084 dedup block) and #44197 (edits lines ~83, 161, 2554–2582) touch completely disjoint ranges. Git's 3-way merge auto-resolves regardless of order; whichever lands second just needs no manual intervention. No blocker.

Minor (non-blocking)

No .npmrc engine-strict=true in the repo, so plain npm install only warns on an unsatisfied engines field rather than hard-failing — so "breaks the build" is slightly stronger than reality for local installs. The pin is still the right defensive fix (stricter CI/electron toolchains do fail, and it prevents silently shipping a node-24-only dep). The hard upper bound <13.11.2 over a caret is intentional and appropriate.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

❌ Lockfile fix failed

See the workflow run for logs.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused dependency and lockfile update. This is already implemented on current main.

  • Automated hermes-sweeper review verified apps/desktop/package.json:64 pins @icons-pack/react-simple-icons to =13.11.1, which excludes the Node >=24 releases.
  • package-lock.json:2781 resolves version 13.11.1 and has no engines.node >=24 constraint.
  • Commit 769f307042d22be2c092249c2d8d78f85fea8e37 (fix(npm): lock react-simple-icons to 13.11.1) made the same manifest and lockfile change; it shipped in v2026.6.19.
  • This also matches the member's duplicate reference to fix: pin @icons-pack/react-simple-icons to ~13.11.1 to avoid EBADENGINE warning #40284.

@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

area/config Config system, migrations, profiles duplicate This issue or pull request already exists 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.

4 participants