Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces a new Zephyr preset for Nitro enabling Cloudflare Zephyr deployment support. It adds preset configuration, runtime server handler, deployment utilities, type definitions, and unit tests, along with the zephyr-agent dependency as an optional peer dependency. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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 |
commit: |
695c9f9 to
2b0464d
Compare
2b0464d to
e3bd861
Compare
# Conflicts: # pnpm-lock.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/unit/zephyr-preset.test.ts (1)
11-36: Consider adding a test for thecompiledhook deployment wiring.Current coverage validates
build:before, but notcompiledbehavior (uploadNitroOutputToZephyrcall path and publicDir resolution). A focused test here would catch deploy-time regressions earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/zephyr-preset.test.ts` around lines 11 - 36, Add a unit test that invokes the zephyr preset's compiled hook to validate deployment wiring: locate the preset via zephyrPresets and its hooks (hooks["compiled"]) and call it with a nitro-like object that stubs options (including output.dir and output.publicDir scenarios), a logger, and a mocked uploadNitroOutputToZephyr function; assert that uploadNitroOutputToZephyr is called with the resolved server output path and the correct publicDir resolution, and that logger methods are used as expected to capture deploy-time messages.src/presets/zephyr/utils.ts (1)
98-109: Provider validation is duplicated and stateful; simplify to a single stateless check.
parsePulledProvideralready enforces the supported provider set, so the laterprovider !== "cloudflare"branch is redundant today. Also, module-levelpulledProvidercouples separate invocations in the same process.Refactor sketch
-let pulledProvider: ZephyrProvider | undefined; @@ -function resolveProvider(appPlatform: unknown): ZephyrProvider { - const pulled = parsePulledProvider(appPlatform); - if (!pulledProvider) { - pulledProvider = pulled; - } - if (pulledProvider !== pulled) { - throw new TypeError( - `[${LOGGER_TAG}] Zephyr PLATFORM changed from "${pulledProvider}" to "${pulled}" within the same process.` - ); - } - return pulledProvider; -} +function resolveProvider(appPlatform: unknown): ZephyrProvider { + return parsePulledProvider(appPlatform); +} @@ - const provider = resolveProvider(appConfig?.PLATFORM); - if (provider !== "cloudflare") { - throw new TypeError( - `[${LOGGER_TAG}] Zephyr PLATFORM "${provider}" is not supported yet by this Nitro preset. Supported today: cloudflare. See https://docs.zephyr-cloud.io` - ); - } + resolveProvider(appConfig?.PLATFORM);Also applies to: 185-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/zephyr/utils.ts` around lines 98 - 109, The resolveProvider function uses a module-level pulledProvider and duplicate validation; remove the stateful pulledProvider usage and simplify resolveProvider to be stateless by returning the result of parsePulledProvider(appPlatform) directly (i.e., call parsePulledProvider and return its value), removing the pulledProvider assignment and the subsequent equality check/TypeError; also apply the same simplification to the similar code block referenced around lines 185-189 so both sites rely solely on parsePulledProvider for validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 181: Replace the wildcard dependency for "zephyr-agent" in package.json
with a pinned semver (e.g. "^0.1.13") and update the lockfile; then verify that
the pinned release exposes the internal APIs used (readDirRecursiveWithContents,
buildAssetsMap, zeBuildDashData, ZephyrEngine.create) by checking the package
source or installed types and run a full build/test to confirm compatibility—if
any API differs, adjust calls in src/presets/zephyr/utils.ts to match the pinned
version or choose a compatible semver range.
---
Nitpick comments:
In `@src/presets/zephyr/utils.ts`:
- Around line 98-109: The resolveProvider function uses a module-level
pulledProvider and duplicate validation; remove the stateful pulledProvider
usage and simplify resolveProvider to be stateless by returning the result of
parsePulledProvider(appPlatform) directly (i.e., call parsePulledProvider and
return its value), removing the pulledProvider assignment and the subsequent
equality check/TypeError; also apply the same simplification to the similar code
block referenced around lines 185-189 so both sites rely solely on
parsePulledProvider for validation.
In `@test/unit/zephyr-preset.test.ts`:
- Around line 11-36: Add a unit test that invokes the zephyr preset's compiled
hook to validate deployment wiring: locate the preset via zephyrPresets and its
hooks (hooks["compiled"]) and call it with a nitro-like object that stubs
options (including output.dir and output.publicDir scenarios), a logger, and a
mocked uploadNitroOutputToZephyr function; assert that uploadNitroOutputToZephyr
is called with the resolved server output path and the correct publicDir
resolution, and that logger methods are used as expected to capture deploy-time
messages.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.jsonsrc/presets/_all.gen.tssrc/presets/_types.gen.tssrc/presets/zephyr/preset.tssrc/presets/zephyr/runtime/server.tssrc/presets/zephyr/utils.tstest/unit/zephyr-preset.test.ts
| "vite": "^7 || ^8 || >=8.0.0-0", | ||
| "xml2js": "^0.6.2" | ||
| "xml2js": "^0.6.2", | ||
| "zephyr-agent": "*" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the actual content of package.json at line 181
sed -n '175,187p' package.jsonRepository: nitrojs/nitro
Length of output: 320
🏁 Script executed:
# Check if src/presets/zephyr/utils.ts exists and see what APIs it uses
if [ -f "src/presets/zephyr/utils.ts" ]; then
echo "=== File found. Checking API calls ==="
cat -n "src/presets/zephyr/utils.ts"
else
echo "=== File not found. Searching for zephyr-related files ==="
find . -type f -path "*/zephyr/*.ts" 2>/dev/null | head -20
fiRepository: nitrojs/nitro
Length of output: 7879
🏁 Script executed:
# Search for any mentions of zephyr-agent in the codebase
rg "zephyr-agent" --type json --type typescript --type javascriptRepository: nitrojs/nitro
Length of output: 91
🌐 Web query:
zephyr-agent npm package documentation compatible version readDirRecursiveWithContents buildAssetsMap
💡 Result:
zephyr-agent is published as an internal package and its npm README only documents the high-level ZephyrAgent API (it explicitly says it’s “not intended for direct use by end users”). It does not mention (or document) readDirRecursiveWithContents or buildAssetsMap, so there’s no “documentation-compatible version” you can reliably target for those symbols from the public docs. [1]
If you’re trying to call those helpers directly, the safest conclusion is that they’re not part of the supported public API of zephyr-agent (i.e., they may be private/internal, moved, or renamed across versions). In that case you should either:
- stay on the supported surface (
ZephyrAgent+ the asset map you build in your plugin), or - locate the exact export in the source repo / generated
.d.tsfor the specific version you’ve installed and pin that exact version.
Also note: a similarly-named, documented assets-map helper exists in zephyr-xpack-internal (buildWebpackAssetsMap), so you may be looking for a function that lives in a different package than zephyr-agent. [2]
Sources: npm zephyr-agent README (v0.0.56) [1]; npm zephyr-xpack-internal README example (buildWebpackAssetsMap) [2].
Pin zephyr-agent to a specific version—the wildcard will break deploys.
The code in src/presets/zephyr/utils.ts calls internal zephyr-agent APIs (readDirRecursiveWithContents, buildAssetsMap, zeBuildDashData, ZephyrEngine.create) that are not documented in the public API and may change across versions. A wildcard dependency cannot provide the stability this integration requires.
The suggested pinning to ^0.1.13 needs to be verified against the actual zephyr-agent package source or type definitions to confirm compatibility with the APIs being used. Check the package repository or installed types to determine the correct semver range.
Suggested change
- "zephyr-agent": "*"
+ "zephyr-agent": "^0.1.13"📝 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.
| "zephyr-agent": "*" | |
| "zephyr-agent": "^0.1.13" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 181, Replace the wildcard dependency for "zephyr-agent"
in package.json with a pinned semver (e.g. "^0.1.13") and update the lockfile;
then verify that the pinned release exposes the internal APIs used
(readDirRecursiveWithContents, buildAssetsMap, zeBuildDashData,
ZephyrEngine.create) by checking the package source or installed types and run a
full build/test to confirm compatibility—if any API differs, adjust calls in
src/presets/zephyr/utils.ts to match the pinned version or choose a compatible
semver range.
This PR adds a built-in preset for Zephyr Cloud
(co-authored with @Nsttt based on ZephyrCloudIO/zephyr-packages#358)
Checklist:
zephyr.providerconfigzephyr-agentSDK with prompt to install