Conversation
Next.js serves page HTML at base path/subpaths (e.g. /p/test-nextjs/) rather than index.html.
Update predicate to match pathname.startsWith(base) && !pathname.includes('/_next/').
PP-2836 fix(nextjs): rewrite response middleware for Next.js page URLs
## [0.14.1-beta.1](v0.14.0...v0.14.1-beta.1) (2026-02-24) ### Bug Fixes * **nextjs:** rewrite response middleware for Next.js page URLs ([3e80f41](3e80f41))
📝 WalkthroughWalkthroughA bug fix release (0.14.1-beta.1) updating the Next.js rewrite response middleware logic to properly exclude Next.js static assets by checking for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli.ts (1)
983-999: Core fix logic is correct — consider adding a test for the new predicateThe predicate
pathname.startsWith(base) && !pathname.includes('/_next/')is well-reasoned:
startsWith(base)is sound becausebasealways ends with/(enforced at Lines 695–697), so it won't spuriously match sibling paths.!includes('/_next/')is necessary for Next.js static assets served under the base prefix (e.g./p/test-nextjs/_next/static/…). Root-level/_next/routes are already short-circuited toessentialMiddlewareChainat Lines 741–745; this guard covers the remaining case.The existing integration tests in
tests/integration/middleware/rewrite-response.spec.tsexerciseinitRewriteResponsewith a standaloneurl.endsWith('index.html')matcher and don't cover the production predicate. A dedicated test for the new logic (both the happy path and the/_next/exclusion) would guard against regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 983 - 999, Add an integration test for the new predicate used when creating rewriteResponseMiddleware (the call to initRewriteResponse) that asserts requests with pathnames starting with base (remember base ends with '/') are rewritten via mi!.buildPage/urlReplacer and that requests containing '/_next/' under the base are excluded; update tests/integration/middleware/rewrite-response.spec.ts to include at least two cases: a positive case where pathname.startsWith(base) triggers the rewrite and a negative case where pathname includes '/_next/' under the base does not trigger the rewrite (and ensure headers/host are set so the same code path is exercised).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli.ts`:
- Around line 983-999: Add an integration test for the new predicate used when
creating rewriteResponseMiddleware (the call to initRewriteResponse) that
asserts requests with pathnames starting with base (remember base ends with '/')
are rewritten via mi!.buildPage/urlReplacer and that requests containing
'/_next/' under the base are excluded; update
tests/integration/middleware/rewrite-response.spec.ts to include at least two
cases: a positive case where pathname.startsWith(base) triggers the rewrite and
a negative case where pathname includes '/_next/' under the base does not
trigger the rewrite (and ensure headers/host are set so the same code path is
exercised).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsontests/test-nextjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mdpackage.jsonsrc/cli.tstests/test-nextjs/pp-dev.config.ts
Summary
Merge
developintomainfor release 0.14.1-beta.1.Changes
Summary by CodeRabbit
Release Notes