cacheable-request - chore: upgrade normalize-url to 9 (breaking)#1681
Conversation
normalize-url ^8.1.1 -> ^9.0.1 (cacheable-request dependency). No code changes required: cacheable-request's usage type-checks against v9 (cacheable-request builds with tsc, so this is enforced by the build). The normalized-URL output that feeds cache keys is unchanged -- the cache-key assertion tests (e.g. "GET:http://mockhttp.org") pass on v9, so cache keys are stable for consumers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1TBuMiXotBjpNHYcbwvqf
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request upgrades the 'normalize-url' dependency from '^8.1.1' to '^9.0.1' in 'packages/cacheable-request/package.json' and updates the lockfile. The review feedback points out a compatibility mismatch: the upgraded dependency requires Node.js '>=20', while the package itself specifies support for Node.js '>=18'. To resolve this, the package's own Node.js engine requirement should be updated to '>=20', or the dependency upgrade should be reverted to maintain compatibility with Node.js 18.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1681 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 29 29
Lines 3504 3504
Branches 810 809 -1
=========================================
Hits 3504 3504 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
normalize-url@9 requires node >=20, but cacheable-request declared engines.node ">=18", so an engine-strict install on Node 18/19 could fail. Raise the floor to ">=20" to match the dependency. Node 18 and 19 are both end-of-life and are not in this repo's CI matrix (22/24/26). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1TBuMiXotBjpNHYcbwvqf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a97dd50cfe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
Maintenance (
chore) — runtime phase, major upgrade ofnormalize-urlincacheable-request. Marked(breaking)for the major bump, though no code changes were needed and cache-key output is unchanged (see below).Versions
normalize-url^8.1.1→^9.0.1(cacheable-request, dependency)Breaking notes
cacheable-request'snormalizeUrl(...)usage type-checks against v9 — and because this package builds withtsc(tsc --project tsconfig.build.json), that's enforced by the build itself.normalize-urlfeeds cache-key generation, so a changed normalization would silently break consumers' caches. The cache-key assertion tests (e.g.GET:http://mockhttp.org, path-stripping, trailing-slash) pass unchanged on v9, confirming the normalized output is the same.Verification
pnpm build(incl. cacheable-request'stscbuild) +node scripts/test-build.mjspasspnpm test—cacheable-requestis 65/66 (the sole failure is the pre-existing sandbox-onlyreturn with url and port, which reaches externalmockhttp.org:8080; passes on GitHub CI). All other packages remain at 100%.Generated by Claude Code