feat: export cliFetch util - #174
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReplaces inline CLI fetch with a new exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli.ts (1)
157-185: Addduplex: "half"for streaming request bodiesWhen
--data@-or `--data `@fileis used, the body is a WHATWGReadableStream. Node.js'sfetch(via Undici) requiresduplex: "half"for streaming request bodies; without it,new Request(...)throwsTypeError: RequestInit: duplex option is required when sending a body.Proposed fix
- const req = new Request(url, { - method, - headers, - body, - }); + const req = new Request( + url, + { + method, + headers, + body, + ...(body instanceof ReadableStream ? { duplex: "half" } : {}), + } as RequestInit & { duplex?: "half" }, + );src/loader.ts (1)
16-31: Handlefile://entries without breaking the notFound checkThe docs say
entrycan be a URL, butresolve()+existsSync()will rejectfile://...inputs and prevent them from loading. The current code passes file URLs directly toresolve(), which treats them as path components, creating invalid paths like./file:///path/to/server.ts. You should detect file URLs, convert them to paths usingfileURLToPathfor the existence check, and preserve the original URL for import.🛠️ Proposed fix
-import { pathToFileURL } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url";- let entry: string | undefined = opts.entry; - if (entry) { - entry = resolve(opts.dir || ".", entry); - if (!existsSync(entry)) { - return { notFound: true }; - } - } else { + let entry: string | undefined = opts.entry; + let entryUrl: string | undefined; + if (entry) { + if (entry.startsWith("file://")) { + entryUrl = entry; + entry = fileURLToPath(entry); + } else { + entry = resolve(opts.dir || ".", entry); + } + if (!existsSync(entry)) { + return { notFound: true }; + } + } else { for (const defEntry of defaultEntries) { for (const defExt of defaultExts) { const entryPath = resolve(opts.dir || ".", `${defEntry}${defExt}`); if (existsSync(entryPath)) { entry = entryPath; break; } } if (entry) break; } if (!entry) { return { notFound: true }; } } // Convert to file:// URL for consistent imports - const url = entry.startsWith("file://") ? entry : pathToFileURL(resolve(entry)).href; + const url = entryUrl ?? pathToFileURL(entry!).href;Also applies to: 90–115
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary by CodeRabbit
Bug Fixes
Improvements
New Features
✏️ Tip: You can customize this high-level summary in your review settings.