fix: resolve package.json from the function's directory in detectZisiBuilder - #8424
fix: resolve package.json from the function's directory in detectZisiBuilder#8424bsplatt92 wants to merge 1 commit into
Conversation
…Builder
readPackageUp expects an options object, so passing func.mainFile as a
string was silently ignored and the lookup fell back to process.cwd().
When netlify dev runs with --cwd from outside the project, no
package.json is found, hasTypeModule is wrongly false, and the
{"type":"commonjs"} marker is never written into the functions-serve
directories - in a "type": "module" project every CJS function bundle
is then parsed as ESM and 500s with "module is not defined in ES
module scope". Resolve from the function's directory instead, matching
@netlify/functions-dev.
Fixes netlify#8423
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan includes up to 4 reviews per rolling hour; 3 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized fix resolves package.json from the function directory so CommonJS function bundles are marked correctly when running from outside the project; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
1 participant
Summary
Fixes #8423.
detectZisiBuilderpassedfunc.mainFile(a string) toreadPackageUp, butread-package-upexpects an options object ({ cwd }). The string was silently ignored and thepackage.jsonlookup fell back toprocess.cwd()— the pre-existing@ts-expect-errorTODO on that line ("We seem to be incorrectly using this function, but it seems to work... Investigate.") was flagging exactly this.When
netlify devruns with--cwd <project>from a directory outside the project (the CLI honors--cwdfor config resolution but neverprocess.chdir()s), nopackage.jsonis found,hasTypeModuleis incorrectlyfalse, and the{"type":"commonjs"}marker is never written into the functions-serve directories. In a project whose rootpackage.jsonhas"type": "module", every zisi/esbuild CJS function bundle is then parsed as ESM and every function invocation 500s withReferenceError: module is not defined in ES module scope.This resolves the lookup from the function's own directory instead, which is exactly what
@netlify/functions-devalready does in its equivalent code path (readPackageUp({ cwd: path.dirname(func.mainFile) })), and lets the@ts-expect-errorbe dropped.Testing
npm run typecheckpasses (the@ts-expect-errorremoval confirms the call now matches the library's types).package.jsonwith"type": "module", v1.tsfunction,node_bundler = "esbuild"): runningnetlify dev --cwd <repo>from an outside directory previously 500'd every function withmodule is not defined in ES module scope; with this change the{"type":"commonjs"}marker is written into each.netlify/functions-serve/<name>/directory and all functions respond normally. Running from inside the repo is unaffected.There are currently no unit tests covering
detectZisiBuilder— happy to add one if you can point me at the preferred harness for this module.🤖 Generated with Claude Code