feat: add edgeone-pages preset with build output api v3#4170
feat: add edgeone-pages preset with build output api v3#4170pi0 merged 10 commits intonitrojs:mainfrom
edgeone-pages preset with build output api v3#4170Conversation
|
Someone is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an EdgeOne Pages Nitro preset: documentation, preset registration and types, a runtime handler, utilities that emit an EdgeOne v3 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
🧹 Nitpick comments (2)
docs/2.deploy/20.providers/edgeone-pages.md (1)
18-22: Consider adding an explicit CLI command example.The CLI section references the
edgeone pages deploycommand in prose but doesn't show it as a code block. Adding a concrete example would improve usability:📝 Suggested improvement
## Using the EdgeOne CLI You can also install the Pages scaffolding tool. For detailed installation and usage, refer to [EdgeOne CLI](https://pages.edgeone.ai/document/edgeone-cli). -Once configured, use the edgeone pages deploy command to deploy the project. During deployment, the CLI will first automatically build the project, then upload and publish the build artifacts. +Once configured, use the following command to deploy the project: + +```bash +edgeone pages deploy +``` + +During deployment, the CLI will first automatically build the project, then upload and publish the build artifacts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/2.deploy/20.providers/edgeone-pages.md` around lines 18 - 22, Add an explicit CLI example showing the command string `edgeone pages deploy` as a code block in the CLI section so readers can copy/paste it; insert a short bash code block containing the single command `edgeone pages deploy` immediately after the prose that mentions it and keep the existing sentence about automatic build/upload/publish directly below the code block for context.src/presets/edgeone/runtime/edgeone.ts (1)
8-12: Interface extends IncomingMessage but redefines existing properties.
IncomingMessagealready hasurl,method, andheaders. This interface narrows them to be non-optional, which is valid if EdgeOne guarantees these fields. Consider if this interface is necessary or if a simple type assertion would suffice.This is fine as-is if documenting the EdgeOne runtime contract is intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/edgeone/runtime/edgeone.ts` around lines 8 - 12, EdgeOneRequest re-declares properties already on IncomingMessage which narrows them to non-optional; either remove EdgeOneRequest and use IncomingMessage directly where possible, or replace the interface with a clearer type that enforces those fields without duplicating (e.g., use a Required/Pick composition like Required<Pick<IncomingMessage, "url"|"method"|"headers">> & IncomingMessage) and update places that reference EdgeOneRequest; ensure references to EdgeOneRequest are updated to the chosen approach so the runtime contract is still expressed without illegally redefining existing IncomingMessage members.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/presets/edgeone/preset.ts`:
- Around line 5-29: The preset defines an EdgeOne Pages bundle but uses the
wrong entry filename; in the edgeone preset created via defineNitroPreset
(symbol: edgeone) update the rollupConfig.output.entryFileNames value from
"handler.js" to the EdgeOne Pages convention "[[default]].js" so that the
generated server function inside serverDir/cloud-functions will be routed
correctly (look for rollupConfig and entryFileNames in the edgeone object).
---
Nitpick comments:
In `@docs/2.deploy/20.providers/edgeone-pages.md`:
- Around line 18-22: Add an explicit CLI example showing the command string
`edgeone pages deploy` as a code block in the CLI section so readers can
copy/paste it; insert a short bash code block containing the single command
`edgeone pages deploy` immediately after the prose that mentions it and keep the
existing sentence about automatic build/upload/publish directly below the code
block for context.
In `@src/presets/edgeone/runtime/edgeone.ts`:
- Around line 8-12: EdgeOneRequest re-declares properties already on
IncomingMessage which narrows them to non-optional; either remove EdgeOneRequest
and use IncomingMessage directly where possible, or replace the interface with a
clearer type that enforces those fields without duplicating (e.g., use a
Required/Pick composition like Required<Pick<IncomingMessage,
"url"|"method"|"headers">> & IncomingMessage) and update places that reference
EdgeOneRequest; ensure references to EdgeOneRequest are updated to the chosen
approach so the runtime contract is still expressed without illegally redefining
existing IncomingMessage members.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5b842d0-d889-4fd6-ad1c-a43749dd08ef
📒 Files selected for processing (6)
docs/2.deploy/20.providers/edgeone-pages.mdsrc/presets/_all.gen.tssrc/presets/_types.gen.tssrc/presets/edgeone/preset.tssrc/presets/edgeone/runtime/edgeone.tssrc/presets/edgeone/utils.ts
| const edgeone = defineNitroPreset( | ||
| { | ||
| extends: "node-server", | ||
| entry: "./edgeone/runtime/edgeone", | ||
| serveStatic: true, | ||
| output: { | ||
| dir: "{{ rootDir }}/.edgeone", | ||
| serverDir: "{{ output.dir }}/cloud-functions/ssr-node", | ||
| publicDir: "{{ output.dir }}/assets", | ||
| }, | ||
| rollupConfig: { | ||
| output: { | ||
| entryFileNames: "handler.js", | ||
| }, | ||
| }, | ||
| hooks: { | ||
| async compiled(nitro: Nitro) { | ||
| await writeEdgeOneConfig(nitro); | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "edgeone-pages" as const, | ||
| } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
EdgeOne Pages cloud functions entry file naming convention handler.js
💡 Result:
In EdgeOne Pages Cloud Functions (Node.js runtime), there is no specific required naming convention mandating "handler.js" as the entry file name. Instead, functions use flexible file naming based on the desired URL route. Cloud Functions are placed in the ./cloud-functions/ directory (sometimes referred to as node-functions in docs). The file name and path determine the access route: - Arbitrary .js names map directly to routes. Examples from official docs: - ./cloud-functions/api/hello.js → example.com/api/hello - ./cloud-functions/api/nodeinfo.js → example.com/api/nodeinfo - ./cloud-functions/index.js → example.com/ - For frameworks like Express/Koa, use [[default]].js as the entry file in a subdirectory (e.g., ./cloud-functions/express/[[default]].js) and export the app instance (export default app;). The entry function must be exported as default (e.g., export default function onRequest(context) {...}), but the file can be named freely except for framework catch-all cases. No sources mention "handler.js" as a convention; examples use descriptive names like hello.js or route-specific names.
Citations:
- 1: https://pages.edgeone.ai/document/node-functions
- 2: https://pages.edgeone.ai/resources/node-functions
- 3: https://express-template.edgeone.app/
- 4: https://pages.edgeone.ai/document/cloud-functions
- 5: https://pages.edgeone.ai/templates/express-template
Use [[default]].js instead of handler.js for the entry file name to match EdgeOne Pages conventions.
The output paths and compiled hook are correct, and extending node-server is appropriate. However, the entryFileNames: "handler.js" doesn't align with EdgeOne Pages naming conventions. EdgeOne Pages routes functions based on file names within the cloud-functions/ directory. For SSR applications, the documented pattern is [[default]].js (used in framework examples like Express templates). Change the rollupConfig to:
rollupConfig: {
output: {
entryFileNames: "[[default]].js",
},
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/edgeone/preset.ts` around lines 5 - 29, The preset defines an
EdgeOne Pages bundle but uses the wrong entry filename; in the edgeone preset
created via defineNitroPreset (symbol: edgeone) update the
rollupConfig.output.entryFileNames value from "handler.js" to the EdgeOne Pages
convention "[[default]].js" so that the generated server function inside
serverDir/cloud-functions will be routed correctly (look for rollupConfig and
entryFileNames in the edgeone object).
pi0
changed the title
edgeone-pages preset with build output api v3
commit: |
The previous two-pass replacement turned '/**' into '/(.([^/]+))' because the single-* pass matched the '*' inside the '(.*)' produced by the first pass, causing static asset routes to bypass the filesystem handler.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/presets/edgeone/utils.ts (1)
34-55: Move internal helper to end of file; consider escaping regex metacharacters.
Per coding guidelines, non-exported helpers should be placed at the end of the file after exported functions.
The function doesn't escape regex metacharacters (
.,+,?,(,),[,],{,},|,^,$,\) in the route before conversion. A route like/api/v1.0/userswould become^/api/v1.0/users$where.matches any character instead of a literal dot.♻️ Proposed fix to escape metacharacters
function routeToRegex(route: string): string { + const escaped = route.replace(/[.+?^${}()|[\]\\]/g, "\\$&"); return ( "^" + - route.replace(/\*\*|\*|:[^/]+/g, (m) => { + escaped.replace(/\*\*|\*|:[^/]+/g, (m) => { if (m === "**") return "(.*)"; return "([^/]+)"; }) + "$" ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/edgeone/utils.ts` around lines 34 - 55, The helper routeToRegex currently sits mid-file and fails to escape regex metacharacters before doing replacements, so literal characters like '.' will be treated as regex tokens; move the non-exported function routeToRegex to the end of the file per guidelines and modify it to first escape RE2 metacharacters (e.g. .+?()[]{}|^$\\) in the incoming route string, then perform the existing replacement of "**", "*", and ":param" tokens to "(.*)" or "([^/]+)" and wrap with "^" and "$" to produce the final regexp string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/presets/edgeone/utils.ts`:
- Around line 110-142: The routes being pushed to config.routes (created from
apiRoutes, the ssrRoutes array, and the final catch-all) lack dest properties so
matches will serve filesystem content instead of forwarding to the SSR/function
handler; update the code that builds sourceRoute in the for loop over apiRoutes,
the object pushed in the for loop over ssrRoutes, and the final catch-all push
to include a dest value (e.g., the name/path of the SSR function or handler
endpoint your platform expects), ensuring apiRoutes->sourceRoute includes dest,
the ssrRoutes push uses dest, and the catch-all push sets dest to the SSR
handler.
---
Nitpick comments:
In `@src/presets/edgeone/utils.ts`:
- Around line 34-55: The helper routeToRegex currently sits mid-file and fails
to escape regex metacharacters before doing replacements, so literal characters
like '.' will be treated as regex tokens; move the non-exported function
routeToRegex to the end of the file per guidelines and modify it to first escape
RE2 metacharacters (e.g. .+?()[]{}|^$\\) in the incoming route string, then
perform the existing replacement of "**", "*", and ":param" tokens to "(.*)" or
"([^/]+)" and wrap with "^" and "$" to produce the final regexp string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d33d313-68d6-473e-91d2-861ed46040b5
📒 Files selected for processing (2)
.gitignoresrc/presets/edgeone/utils.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/presets/edgeone/preset.ts (1)
9-9: Remove the inline explanatory comment in config.Keep this line comment-free per repo style rules.
Suggested change
- serveStatic: false, // EdgeOne serves static assets from `.edgeone/assets/` + serveStatic: false,As per coding guidelines, "Do not add comments explaining what the line does unless prompted".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/edgeone/preset.ts` at line 9, Remove the inline explanatory comment on the serveStatic property in preset.ts: locate the serveStatic: false, entry in the exported preset object and delete the trailing comment text ("// EdgeOne serves static assets from `.edgeone/assets/`") so the line is comment-free while preserving the property and punctuation.
🤖 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/presets/edgeone/preset.ts`:
- Line 9: Remove the inline explanatory comment on the serveStatic property in
preset.ts: locate the serveStatic: false, entry in the exported preset object
and delete the trailing comment text ("// EdgeOne serves static assets from
`.edgeone/assets/`") so the line is comment-free while preserving the property
and punctuation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2265bfce-cbe8-4bca-a7aa-5bd59da2cdbf
📒 Files selected for processing (2)
src/presets/_types.gen.tssrc/presets/edgeone/preset.ts
✅ Files skipped from review due to trivial changes (1)
- src/presets/_types.gen.ts
2 participants
🔗 Linked issue
#2973
Supersedes #3884
❓ Type of change
📚 Description
Hi, I'm a developer of EdgeOne Pages. This PR adds a preset to support deploying Nitro projects on EdgeOne Pages.
This is a rewrite of my previous PR. Based on the feedback and discussions from the previous review, we identified some issues with the build output structure. The key changes in this version include:
config.jsonformat instead of the previousmeta.jsonapproach.This is our build documentation. If you have any questions, feel free to discuss with us.
The implementation and deployment testing have been completed.
Please review. Thanks.
📝 Checklist