feat(vercel): rewrite proxy route rule on cdn#4006
Conversation
|
@RihanArfan is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR implements support for offloading proxy route rules to Vercel's CDN-level rewrites. It adds logic to determine which proxies are eligible for CDN handling, updates type definitions, modifies the build configuration generation, and includes documentation and tests demonstrating the feature with an external CDN proxy example. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
commit: |
| return false; | ||
| } | ||
| // Must be an external URL | ||
| if (!/^https?:\/\//.test(proxy.to.replace(/\/\*\*$/, ""))) { |
There was a problem hiding this comment.
Tip: We could also use URL.canParse but this is also good!
pi0
changed the title
proxy route rule on cdn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/tests.ts`:
- Around line 547-554: The "external proxy" test calls callHandler which
performs a real network request and must be skipped when offline; update the
test block named "external proxy" to check process.env.OFFLINE (same pattern
used by other tests) and skip the test when that variable is set (e.g., use the
test.skip pattern or return early when OFFLINE is truthy) so the callHandler
network request is not executed in offline CI environments.
| it("external proxy", async () => { | ||
| const { data, headers, status } = await callHandler({ | ||
| url: "/cdn/npm/bootstrap@5.3.8/dist/js/bootstrap.min.js", | ||
| }); | ||
| expect(status).toBe(200); | ||
| expect(headers["etag"]).toMatch(/W\/".+"/); | ||
| expect(data).toContain("Bootstrap"); | ||
| }); |
There was a problem hiding this comment.
Test depends on external network access but lacks an OFFLINE skip guard.
This test makes a real HTTP request to cdn.jsdelivr.net. Other tests in this file (Line 752) already use process.env.OFFLINE to skip when network is unavailable. This test should do the same to avoid CI failures in offline/restricted environments.
Proposed fix
- it("external proxy", async () => {
+ it.skipIf(!!process.env.OFFLINE)("external proxy", async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("external proxy", async () => { | |
| const { data, headers, status } = await callHandler({ | |
| url: "/cdn/npm/bootstrap@5.3.8/dist/js/bootstrap.min.js", | |
| }); | |
| expect(status).toBe(200); | |
| expect(headers["etag"]).toMatch(/W\/".+"/); | |
| expect(data).toContain("Bootstrap"); | |
| }); | |
| it.skipIf(!!process.env.OFFLINE)("external proxy", async () => { | |
| const { data, headers, status } = await callHandler({ | |
| url: "/cdn/npm/bootstrap@5.3.8/dist/js/bootstrap.min.js", | |
| }); | |
| expect(status).toBe(200); | |
| expect(headers["etag"]).toMatch(/W\/".+"/); | |
| expect(data).toContain("Bootstrap"); | |
| }); |
🤖 Prompt for AI Agents
In `@test/tests.ts` around lines 547 - 554, The "external proxy" test calls
callHandler which performs a real network request and must be skipped when
offline; update the test block named "external proxy" to check
process.env.OFFLINE (same pattern used by other tests) and skip the test when
that variable is set (e.g., use the test.skip pattern or return early when
OFFLINE is truthy) so the callHandler network request is not executed in offline
CI environments.
2 participants
🔗 Linked issue
Resolves #3484
❓ Type of change
📚 Description
📝 Checklist