fix: accept decentraland-crypto-fetch 3 as a peer

[LautaroPetaccio]

The problem

The peer was pinned to ^2.0.1, so a consumer moving to the 3.x signer could satisfy neither range and every install failed with ERESOLVE. That's the whole problem this PR exists for.

This package uses exactly two things from that dependency — signedFetchFactory and the AuthIdentity type — and neither changed across the major.

The change

The entire package.json diff:

  peerDependencies:
-   "decentraland-crypto-fetch": "^2.0.1"
+   "decentraland-crypto-fetch": ">=2.0.0"

  devDependencies:
-   "decentraland-crypto-fetch": "^2.0.1"
+   "decentraland-crypto-fetch": "^3.0.0"

A peer range states what this package can work with, not what a consumer should choose. The consumer picks the version; pinning a major on their behalf is what caused the failure.

The dev dependency moves to 3.x so CI tests against the version npm will actually resolve for a consumer, rather than the one it won't.

No source changes. The rest of the diff is the lockfile and three doc lines.

Why it stays required, not optional

An earlier revision marked the peer optional and added a runtime deferral to make that claim true. Rolled back — more machinery than the problem justifies.

Worth recording, since "optional" is superficially attractive here: src/index.ts re-exports the notifications client through ./clients, and ESM resolves static re-exports at link time. So importing any hook resolves the signer whether or not it's used. Declaring it optional without deferring that import would advertise a contract the code doesn't honour — and deferring it means a dynamic import(), a memoised loader, and a module-graph test to keep it honest.

Required is the honest description of what the code does today.

The consequence, stated plainly

npm resolves a peer to the highest satisfying version, so consumers get 3.x, which declares node >= 22. Most consumers of this package target Node 18 or 20:

Node	consumers
18	landing
20	account, auth, blog-site, profile
20 ∥ 22+	explorer-website, sites
22+	decentraland-dapps

They'll see EBADENGINE on install — a warning, or a hard failure under engine-strict. These are browser apps, so nothing breaks at runtime; the engines field affects install and CI only.

An app that must stay on 2.x should depend on decentraland-crypto-fetch directly rather than relying on this range to hold it back.

Checks

14 suites / 190 tests pass. tsc --noEmit and npm run build clean.

[decentraland-bot]

Requesting changes for one package-contract issue.

Findings:

• P1 — package.json:22 now advertises decentraland-crypto-fetch@^3.0.0 as an acceptable required peer, but v3 declares node >=22.0.0 (package-lock.json:3350). This package does not publish a matching engines requirement and its docs say the peer is only needed for useNotifications. With npm peer auto-installation, consumers that do not pin decentraland-crypto-fetch can resolve v3 and fail installs under Node 18/20 + engine-strict, including consumers that do not use notifications. Please either make this peer optional via peerDependenciesMeta if it is notifications-only, or explicitly raise/document the package’s Node support as part of the contract.
• P2 — consumer-facing docs are stale/inconsistent with the new peer range: README.md:16 says >= 2.0.1, docs/useNotifications.md:18 and AGENTS.md:5 still say ^2.0.1. Please update them to the exact supported range and mention the Node 22 requirement for v3.
• P2 — the published peer range claims both v2 and v3 compatibility, but CI now only installs/tests v3. A small package-contract check for decentraland-crypto-fetch@2.0.1 would keep the dual-range guarantee from regressing.
• Security: no security issues found.
• CI: passing (audit, build, test).

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack

[decentraland-bot]

Thanks for the updates. The original peer-range conflict is addressed, and CI is green, but the new optional-peer contract still has one blocking package-contract issue.

Findings:

• P1 — package.json now marks decentraland-crypto-fetch optional, but the root entrypoint still statically reaches it through src/index.ts -> src/clients/index.ts -> src/clients/notifications/createNotificationsClient.ts. In ESM, static re-exports are resolved during module linking, so a consumer that imports unrelated hooks from @dcl/hooks without installing the optional peer can still fail to bundle/start even if it never calls notifications. That makes the new optional-peer contract misleading and can break consumers that rely on the package metadata. Please either keep the peer required, or truly isolate notifications behind a notification-only subpath/dynamic import, and ideally add a smoke test that imports a non-notification hook from a packed install without decentraland-crypto-fetch.
• P2 — package-lock.json is out of sync with package.json: the root lockfile entry has peerDependencies but not the new peerDependenciesMeta optional metadata. Please regenerate/update the lockfile so the published package contract and lockfile match.
• P2 — test/package-contract.test.ts imports semver, but semver is not declared as a direct dev dependency. Please add it explicitly (and types if needed), or simplify the test to assert the exact intended range string.
• P2 — docs/useNotifications.md says the optional peer “is only installed if you use this hook.” Package managers do not install optional peers based on hook usage; consumers must install the signer themselves when using notifications. Please reword this to “Install this optional peer only in apps that use useNotifications / createNotificationsClient.”
• Security: no secrets, auth bypass, or signed-fetch security issue found in the changed code/dependency contract.
• CI: passing (audit, build, test).

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack

[LautaroPetaccio]

All four addressed.

P1 — the optional contract is now real

You're right, and the diagnosis was exact: src/index.ts → ./clients → createNotificationsClient.ts was a static chain, so ESM linking resolved decentraland-crypto-fetch for anyone importing any hook at all.

Fixed by deferring the value import, not by adding a subpath — that keeps createNotificationsClient's synchronous signature and the returned client's shape unchanged, so no consumer has to move. It works because both client methods were already async; awaiting the module inside them changes nothing observable.

• AuthIdentity stays a type-only import (erased), and SignedFetch is taken via a type-position import() for the same reason.
• The promise is cleared on failure rather than cached: a missing peer is permanent and the consumer must fix it, but a transient chunk-load failure shouldn't poison every later call.
• The thrown error names the peer and who needs it — the bare resolution error names a module the consumer never imported.

Guarded, and the guard has a guard

test/optional-peer-isolation.test.ts walks the static import graph from src/index.ts and asserts nothing on it reaches the peer. On source, deliberately: CI runs npm ci then jest with no build step, so there's nothing in esm/ to read — and source is where the mistake would be made.

Two companion assertions keep it honest:

• the client still reaches the peer dynamically, so deleting the client couldn't satisfy the first assertion
• the walk actually reaches the client, so it can't pass vacuously

That third one earned its place immediately: it caught my walker resolving relative specifiers against the wrong directory, silently truncating the graph to one hop. Without it I'd have shipped a guard that passed because it inspected almost nothing.

Counterfactual: restoring the static import fails the first assertion and names the offending module.

On the packed-install smoke test

I ran it, and it surfaced something worth knowing. With the tarball installed and no decentraland-crypto-fetch present, import '@dcl/hooks' under plain Node ESM fails — but with ERR_UNSUPPORTED_DIR_IMPORT on esm/clients, not a missing peer. The emitted output uses extensionless directory re-exports (export * from "./clients"), which is on main too and is inherent to moduleResolution: bundler. Pre-existing packaging shape, out of scope here — this package is consumed through bundlers, which resolve those fine.

So a raw-Node smoke test would assert the wrong thing. I verified the artifact property directly instead: in the packed, peer-less install there is no static edge to decentraland-crypto-fetch anywhere under esm/ — only the dynamic import(...) in the client. That's the guarantee you asked for; the jest graph walk is what prevents regression without adding a bundler to CI.

P2 — lockfile out of sync

Regenerated. The root entry now records peerDependenciesMeta: {"decentraland-crypto-fetch":{"optional":true}} alongside peerDependencies.

P2 — undeclared semver

Declared semver@^7.7.1 and @types/semver@^7.5.8 as devDependencies. Worth noting how undeclared it was: the test was resolving a hoisted transitive 6.3.1, and @types/semver was phantom too — so the test's meaning depended on another package's hoisting.

I kept satisfies() rather than switching to a string equality: it tests the contract (does this range admit 2.x and 3.x) rather than its spelling, which is what the test is for.

P2 — misleading doc

Reworded. It now says plainly that installing @dcl/hooks will not pull the peer in, that you install it yourself in apps using useNotifications / createNotificationsClient, and that package managers can't tell which hooks an app calls. Also added why apps that use neither can now leave it out — which is only true because of the P1 fix.

Checks

15 suites / 174 tests pass. tsc --noEmit clean. eslint and prettier clean on every touched file.

Pre-existing and untouched: 3 eslint errors in src/hooks/useAdvancedUserAgentData/useAdvancedUserAgentData.ts (verified present on the pristine tree) and prettier drift across ~14 docs files. Left alone rather than folding an unrelated reformat into this diff.

[decentraland-bot]

Re-review complete. The previous blocking optional-peer issue is addressed: decentraland-crypto-fetch is now an optional peer, the runtime value import is deferred until notification methods are called, and the new tests guard both the package contract and the root static import graph.

Findings:

• P2 — README.md:16 still says the optional peer is only needed for useNotifications, but the public createNotificationsClient export also needs it when used directly. Please include createNotificationsClient in that sentence for consistency with docs/useNotifications.md.
• P2 — The source graph test is useful but intentionally regex-based and source-only, so it could miss less common static dependency forms (require(...), TS import-equals, aliased internal imports). Not blocking for this change, but a packed-consumer/bundler smoke test would be stronger if this contract regresses again.

Consumer impact: public API shape is unchanged and the peer range is backward-compatible (^2.0.1 || ^3.0.0), with the missing optional peer now failing only when the notifications client is actually used.

Security: no new security issues found in the changed code or dependency contract.

CI: GitHub checks are passing (audit, build, test). I also verified locally with npm test -- --runInBand (174 passing) and npm run build.

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack