Changes to make teams work #432
Conversation
WalkthroughAdds a hidden Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant Dev as dev cmd
participant B as Bundler
participant S as Server
U->>Dev: run dev [--no-build]
alt no-build = false
Dev->>B: build(initial=true)
alt esbuild errors (DevMode)
B-->>Dev: ErrBuildFailed (ctx.Logger.Debug logged)
Dev->>Dev: stop spinner, continue (no exit)
else other bundling error
B-->>Dev: error -> exit (ErrInvalidConfiguration)
else success
B-->>Dev: ok
Dev->>S: start server
end
else no-build = true
Dev->>Dev: log "Skipping build (no-build flag set)"
Dev->>S: start server (skip initial build)
end
loop file changes
Dev->>B: build(initial=false) [if building enabled]
B-->>Dev: ok / ErrBuildFailed / error
alt ok
Dev->>S: restart server
else ErrBuildFailed
Dev->>Dev: stop spinner, continue (no exit)
else other error
Dev->>Dev: exit with configuration error
end
end
sequenceDiagram
participant U as User
participant Cloud as cloud cmd
participant P as Preflight
participant B as Bundler
participant D as Deployer
U->>Cloud: run deploy [--no-build]
Cloud->>P: PreflightCheck(..., noBuild)
alt noBuild = false
P->>B: bundle project
alt bundle error
B-->>P: error -> P returns error
else success
B-->>P: ok -> P returns mutator
end
else noBuild = true
P-->>Cloud: skip bundling, return mutator
end
Cloud->>D: proceed with deploy
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/bundler/bundler.go (1)
298-301: Reduce debug log verbosity; summarize esbuild errors instead of dumping the full sliceIn DevMode you already print nicely formatted errors to
ctx.Writer. Logging the entireresult.Errorsslice can be very noisy and hard to read. Prefer logging a concise summary (e.g., count) here.Apply this diff:
- ctx.Logger.Debug("build failed: %v", result.Errors) + ctx.Logger.Debug("build failed (%d errors)", len(result.Errors))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
cmd/dev.go(4 hunks)internal/bundler/bundler.go(1 hunks)internal/bundler/shim.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
cmd/dev.go (4)
43-45: Examples updated for --no-build flag look goodNice addition to the usage examples; this improves discoverability of the new workflow.
49-49: Flag retrieval is correct
noBuild, _ := cmd.Flags().GetBool("no-build")aligns with the declared flag and is used consistently below.
198-202: Initial build gating matches the new flag’s intentSkipping the initial build when
--no-buildis set is correct, and returning early on a failed initial build keeps UX consistent.
274-275: Flag registration looks correctThe new
--no-buildflag is properly declared with defaults and help text.
…nto teams-changes
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
cmd/cloud.go(3 hunks)internal/bundler/node-fetch.go(1 hunks)internal/bundler/patch.go(2 hunks)internal/deployer/deployer.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/deployer/deployer.go (2)
internal/util/io.go (1)
ZipDirCallbackMutator(108-108)internal/bundler/bundler.go (1)
Bundle(426-453)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (12)
internal/bundler/patch.go (5)
104-104: LGTM - Well-designed JavaScript file detection.The
isJSflag usingstrings.HasSuffix(args.Path, ".js")provides a clean way to differentiate JavaScript from TypeScript files for loader selection.
108-116: Excellent enhancement to support const variable declarations.The addition of
isConstVariabledetection significantly improves the patching system by supporting both function declarations (function foo()) and const variable function assignments (const foo = ()). The fallback logic is well-implemented with proper early exit when neither pattern is found.
125-130: Well-structured function name transformation logic.The conditional logic for generating
newfnnamebased onisConstVariablecorrectly handles both function declaration and const variable assignment patterns.
139-144: Smart differentiation between JS and TypeScript wrapper generation.The wrapper generation logic appropriately handles the differences between JavaScript and TypeScript:
- JavaScript: Uses arrow function syntax and
argumentsobject- TypeScript: Uses traditional function syntax with rest parameters
This addresses the different argument handling capabilities between the two languages.
174-176: Correct loader selection based on file type.The loader selection logic properly defaults to
LoaderJSand switches toLoaderTSfor non-JavaScript files, aligning with the file detection logic.internal/bundler/node-fetch.go (1)
3-17: Well-implemented node-fetch AbortSignal patch.The patch correctly enhances the
isAbortSignalfunction with additional logic to handle AbortSignal detection. TheAfterhook implementation:
- First checks if the original function already returned
true- Falls back to constructor name checking for AbortSignal objects
- Properly accesses the first argument via
_args[0]This is a solid runtime enhancement that maintains backward compatibility while extending functionality.
cmd/cloud.go (3)
201-201: LGTM - Proper flag extraction.The
noBuildflag is correctly extracted from command line arguments using the standard cobra flag parsing pattern.
312-312: Correct integration with PreflightCheck.The
noBuildparameter is properly passed toPreflightCheck, enabling conditional build behavior based on the command line flag. This aligns with the updated function signature ininternal/deployer/deployer.go.
1020-1020: Good flag definition with appropriate default.The
--no-buildflag is properly defined with:
- Appropriate boolean type
- Sensible default value (
false)- Clear description of functionality
internal/deployer/deployer.go (3)
71-71: Good API design for conditional building.The addition of the
noBuild boolparameter toPreflightCheckprovides a clean way to control build behavior. The parameter is appropriately positioned at the end to maintain backward compatibility concerns.
73-80: Well-structured bundler context initialization.The
BundleContextinitialization is comprehensive and includes all necessary fields:
Loggerfor debugging and error reportingProjectDirfor project locationProduction: truefor production builds- Proper context and writer setup
81-85: Clean conditional build logic.The conditional bundling implementation is straightforward and effective:
- Only runs bundling when
noBuildis false- Preserves error handling when bundling occurs
- Maintains the existing error propagation pattern
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/bundler/patch.go (1)
139-141: Fix the duplicated prefix construction.The current implementation duplicates the prefix by concatenating "export " with an existing
fnprefixthat may already contain "async ", resulting in malformed prefixes like "export async async ".Apply this diff to fix the prefix concatenation:
- if isExport { - fnprefix += "export " + fnprefix - } + if isExport { + fnprefix = "export " + fnprefix + }
🧹 Nitpick comments (1)
internal/bundler/node-fetch.go (1)
5-6: Remove debug print statement.The debug print statement should be removed from production code as it will clutter the output during normal operations.
Apply this diff to remove the debug output:
-func init() { - fmt.Println("patching node-fetch")Alternatively, if debugging output is needed, consider using the logger instead:
- fmt.Println("patching node-fetch") + // Patch registration for node-fetch module
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/bundler/node-fetch.go(1 hunks)internal/bundler/patch.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (7)
internal/bundler/patch.go (6)
105-105: LGTM! Good file type detection logic.The
isJSflag properly distinguishes between JavaScript and TypeScript files based on the file extension, which drives the appropriate code path selection throughout the patching process.
109-117: LGTM! Good fallback handling for different function declaration patterns.The code properly handles both traditional function declarations (
function fname) and const variable function expressions (const fname =). The fallback mechanism ensures comprehensive coverage of different JavaScript/TypeScript function declaration patterns.
124-127: LGTM! Good detection of function modifiers and debugging output.The detection logic for
isExportandisArrowis correct and the debugging output will help with troubleshooting patching issues.
129-134: LGTM! Proper handling of different function naming patterns.The conditional logic correctly generates the appropriate function name format based on whether it's a const variable declaration or a traditional function declaration.
143-152: LGTM! Good wrapper generation for different syntaxes.The wrapper generation correctly handles different scenarios:
- JS files with arrow functions get arrow function wrappers
- JS files with regular functions get regular function wrappers
- Non-JS (TypeScript) files get traditional function wrappers with proper parameter destructuring
- Proper argument handling with
let args = arguments;for JS and(...args)for TypeScript
182-184: LGTM! Correct loader selection based on file type.The loader selection properly defaults to
LoaderJSand switches toLoaderTSfor non-JS files, which aligns with the file type detection logic.internal/bundler/node-fetch.go (1)
7-21: Please manually verify the node-fetchisAbortSignalpatchI wasn’t able to locate the upstream
isAbortSignalimplementation locally, so please confirm against node-fetch’s current source:• Inspect the original
isAbortSignalinnode-fetch/lib/index.jsto ensure yourAfterlogic preserves intended behavior
• Remove or replaceconsole.log(_args[0])before shipping to production
• Verify that treating any object as an abort signal won’t introduce false positives
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
internal/bundler/patch.go (1)
132-149: Fix export/async prefix construction and generate valid wrapper headers for function vs variable declarations
- Current logic can duplicate tokens and place
asyncin an invalid position (e.g.,async const foo = () => {), and will emit invalid JS for non-arrow variable declarations (const foo = () {).- Build headers explicitly for each case: function declaration vs. variable declaration, JS vs. TS-like, and properly place
export,default, andasync.Apply this diff to replace the prefix/header logic:
- var fnprefix string - if isAsync { - fnprefix = "async " - } - if isExport { - fnprefix += "export " + fnprefix - } - contents = strings.Replace(contents, fnname, newfnname, 1) - if isJS { - if isArrow { - suffix.WriteString(fnprefix + fnname + "() => {\n") - } else { - suffix.WriteString(fnprefix + fnname + "() {\n") - } - suffix.WriteString("let args = arguments;\n") - } else { - suffix.WriteString(fnprefix + fnname + "(...args) {\n") - } + // Replace the original function name with the internal one + contents = strings.Replace(contents, fnname, newfnname, 1) + + // Build wrapper header with correct export/async placement + var exportPrefix string + if isExport { + if isDefault && !isVarDecl { + exportPrefix = "export default " + } else { + exportPrefix = "export " + } + } + if isJS { + if isVarDecl { + header := exportPrefix + declKeyword + " " + fn + " = " + if isAsync { + header += "async " + } + // Use function expression to preserve `arguments` and `this` + suffix.WriteString(header + "function() {\n") + } else { + header := exportPrefix + if isAsync { + header += "async " + } + suffix.WriteString(header + "function " + fn + "() {\n") + } + suffix.WriteString("let args = arguments;\n") + } else { + if isVarDecl { + header := exportPrefix + declKeyword + " " + fn + " = " + if isAsync { + header += "async " + } + if isArrow { + suffix.WriteString(header + "(...args) => {\n") + } else { + suffix.WriteString(header + "function(...args) {\n") + } + } else { + header := exportPrefix + if isAsync { + header += "async " + } + suffix.WriteString(header + "function " + fn + "(...args) {\n") + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/bundler/node-fetch.go(1 hunks)internal/bundler/patch.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/bundler/node-fetch.go
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cmd/dev.go (1)
147-176: Nil-function panic concern resolved; refine skip-build logging and consider no-op build binding.
- The earlier panic risk due to a nil build func is resolved by always assigning the closure. Good.
- Current implementation logs “Skipping build…” on every restart in no-build mode, which can get noisy.
Minimal tweak to reduce log noise (log once on initial, debug thereafter):
- if noBuild { - log.Info("Skipping build (no-build flag set)") - return true - } + if noBuild { + if initial { + log.Info("Skipping build (no-build flag set)") + } else { + log.Debug("Skipping build (no-build flag set)") + } + return true + }Optional readability refactor: bind a no-op build function when disabled, removing the need to branch at every call:
- var build func(initial bool) bool - - build = func(initial bool) bool { - if noBuild { - log.Info("Skipping build (no-build flag set)") - return true - } - started := time.Now() - var ok bool - tui.ShowSpinner("Building project ...", func() { - if err := bundler.Bundle(bundler.BundleContext{ - Context: ctx, - Logger: log, - ProjectDir: dir, - Production: false, - DevMode: true, - Writer: os.Stdout, - }); err != nil { - if err == bundler.ErrBuildFailed { - return - } - errsystem.New(errsystem.ErrInvalidConfiguration, err, errsystem.WithContextMessage(fmt.Sprintf("Failed to bundle project: %s", err))).ShowErrorAndExit() - } - ok = true - }) - if ok && !initial { - log.Info("✨ Built in %s", time.Since(started).Round(time.Millisecond)) - } - return ok - } + if noBuild { + log.Info("Skipping build (no-build flag set)") + } + build := func(initial bool) bool { return true } // no-op when --no-build + if !noBuild { + build = func(initial bool) bool { + started := time.Now() + var ok bool + tui.ShowSpinner("Building project ...", func() { + if err := bundler.Bundle(bundler.BundleContext{ + Context: ctx, + Logger: log, + ProjectDir: dir, + Production: false, + DevMode: true, + Writer: os.Stdout, + }); err != nil { + if err == bundler.ErrBuildFailed { + return + } + errsystem.New(errsystem.ErrInvalidConfiguration, err, errsystem.WithContextMessage(fmt.Sprintf("Failed to bundle project: %s", err))).ShowErrorAndExit() + } + ok = true + }) + if ok && !initial { + log.Info("✨ Built in %s", time.Since(started).Round(time.Millisecond)) + } + return ok + } + }To ensure no-build mode doesn’t break runtime (e.g., if the run command expects built artifacts), please run:
#!/bin/bash # Locate CreateRunProjectCmd and inspect how the project is launched. rg -n "CreateRunProjectCmd" -A 80 internal | sed -n '1,200p' # Look for indications that runtime depends on build outputs (e.g., dist/, .bundle/) rg -n "dist/|build/|bundle|esbuild|outdir|outFile" internal -S -A 3 # Check for direct TS runners (tsx/ts-node) that would be compatible with --no-build rg -n "ts-node|tsx|node --loader|bun run" internal -S -A 3
🧹 Nitpick comments (1)
cmd/dev.go (1)
43-44: Avoid documenting a hidden flag in Examples (inconsistency).You’re marking --no-build as hidden but showcasing it in the Examples section. This is confusing for users relying on help output.
Apply this diff to remove the hidden flag from the examples (or unhide the flag in init):
Examples: agentuity dev - agentuity dev --dir /path/to/project - agentuity dev --no-build + agentuity dev --dir /path/to/project
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
cmd/cloud.go(3 hunks)cmd/dev.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/cloud.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/dev.go (1)
internal/util/api.go (1)
GetURLs(262-281)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
internal/bundler/patch.go (1)
91-96: Fix closure-capture bug for loop variables used inside OnLoad callbackEach OnLoad closure currently captures the same
name/modfrom the last iteration. Rebind them per-iteration to avoid wrong module being rewritten at runtime.- for name, mod := range patches { - path := "node_modules/" + mod.Module + "/.*" + for name, mod := range patches { + // Rebind loop vars so each closure captures its own values + name := name + mod := mod + path := "node_modules/" + mod.Module + "/.*" if mod.Filename != "" { path = "node_modules/" + mod.Module + "/" + mod.Filename + ".*" } build.OnLoad(api.OnLoadOptions{Filter: path, Namespace: "file"}, func(args api.OnLoadArgs) (api.OnLoadResult, error) {
♻️ Duplicate comments (5)
internal/bundler/patch.go (5)
104-104: Detect all common JS/TS extensions; avoid misclassifying .mjs/.cjs/.jsx/.mts/.ctsOnly checking “.js” will send
.mjs/.cjs/.jsxdown the TS path and break bundling; similarly.mts/.ctsshould be TS. Classify by full extension set.- isJS := strings.HasSuffix(args.Path, ".js") + // Detect script type by extension + isJS := strings.HasSuffix(args.Path, ".js") || + strings.HasSuffix(args.Path, ".mjs") || + strings.HasSuffix(args.Path, ".cjs") || + strings.HasSuffix(args.Path, ".jsx") + isTS := strings.HasSuffix(args.Path, ".ts") || + strings.HasSuffix(args.Path, ".tsx") || + strings.HasSuffix(args.Path, ".mts") || + strings.HasSuffix(args.Path, ".cts")
181-184: Select esbuild loader based on TS-family check (not just “not JS”)Use LoaderTS only for TS-family extensions; keep JS-family on LoaderJS even when not “.js”.
- loader := api.LoaderJS - if !isJS { - loader = api.LoaderTS - } + loader := api.LoaderJS + if isTS { + loader = api.LoaderTS + }
108-116: Generalize detection: support const/let/var variable function declarationsCurrent logic only recognizes
const fn =. Extend toletandvar, and track the keyword for later emission.- var isConstVariable bool + var isVarDecl bool + var declKeyword string if index == -1 { - fnname = "const " + fn + " = " - index = strings.Index(contents, fnname) - isConstVariable = true - if index == -1 { - continue - } + for _, kw := range []string{"const", "let", "var"} { + candidate := kw + " " + fn + " = " + if idx := strings.Index(contents, candidate); idx != -1 { + fnname = candidate + index = idx + isVarDecl = true + declKeyword = kw + break + } + } + if index == -1 { + continue + } }
125-130: Preserve original var-decl kind when renaming the implementationCarry through
const/let/varwhen building the renamed symbol.- var newfnname string - if isConstVariable { - newfnname = "const " + newname + " = " - } else { - newfnname = "function " + newname - } + var newfnname string + if isVarDecl { + newfnname = declKeyword + " " + newname + " = " + } else { + newfnname = "function " + newname + }
123-137: Fix prefix construction and wrapper emission: preserveexport default, placeasynccorrectly, and generate valid headers for var-decls
- Current
fnprefixlogic can emit duplicated tokens and mis-orderexport/async.export defaultis lost, changing module semantics.- Non-JS var-decl wrappers output
const fn = (...args) {(missing=>) and placeasyncbeforeconst, which is invalid.Refactor to compute
exportPrefix/asyncPrefixand emit correct headers for both function and variable declarations.- isAsync := strings.Contains(prefix, "async") - isExport := strings.Contains(prefix, "export") + isAsync := strings.Contains(prefix, "async") + isExport := strings.Contains(prefix, "export") + isDefault := strings.Contains(prefix, "default") newname := "__agentuity_" + fn - var newfnname string - if isConstVariable { - newfnname = "const " + newname + " = " - } else { - newfnname = "function " + newname - } - var fnprefix string - if isAsync { - fnprefix = "async " - } - if isExport { - fnprefix += "export " + fnprefix - } + var newfnname string + if isVarDecl { + newfnname = declKeyword + " " + newname + " = " + } else { + newfnname = "function " + newname + } + // Build prefixes + var exportPrefix, asyncPrefix string + if isExport { + exportPrefix = "export " + if isDefault { + exportPrefix += "default " + } + } + // For function declarations, "async" prefixes the function. + // For variable declarations, "async" goes after "=" (see below). + if !isVarDecl && isAsync { + asyncPrefix = "async " + } contents = strings.Replace(contents, fnname, newfnname, 1) - if isJS { - suffix.WriteString(fnprefix + "function " + fn + "() {\n") - suffix.WriteString("let args = arguments;\n") - } else { - suffix.WriteString(fnprefix + fnname + "(...args) {\n") - } + if isJS { + suffix.WriteString(exportPrefix + asyncPrefix + "function " + fn + "() {\n") + suffix.WriteString("let args = arguments;\n") + } else { + if isVarDecl { + if isAsync { + suffix.WriteString(exportPrefix + declKeyword + " " + fn + " = async (...args) => {\n") + } else { + suffix.WriteString(exportPrefix + declKeyword + " " + fn + " = (...args) => {\n") + } + } else { + suffix.WriteString(exportPrefix + asyncPrefix + "function " + fn + "(...args) {\n") + } + }Also applies to: 139-144
🧹 Nitpick comments (3)
internal/bundler/patch.go (3)
105-106: Avoid confusing shadowing: rename innermod(patchAction) toactionShadowing
mod(patchModule) withmod(patchAction) harms readability and is error-prone. Rename the inner variable and its uses.- for fn, mod := range mod.Functions { + for fn, action := range mod.Functions { @@ - if mod.Before != "" { - suffix.WriteString(mod.Before) + if action.Before != "" { + suffix.WriteString(action.Before) suffix.WriteString("\n") } @@ - if mod.After != "" { - suffix.WriteString(mod.After) + if action.After != "" { + suffix.WriteString(action.After) suffix.WriteString("\n") }Also applies to: 146-149, 164-167
138-138: Replace targeted substring at the computed index to avoid accidental earlier replacement
strings.Replace(..., 1)will replace the first occurrence, which might not be the one atindexif the pattern appears earlier. Replace at the exact index for correctness.- contents = strings.Replace(contents, fnname, newfnname, 1) + contents = contents[:index] + newfnname + contents[index+len(fnname):]
92-96: Nit: rename localpathvar tofilterto avoid shadowing the importedpathpackagePure readability; avoids confusion between
path(string) andpath(package).- path := "node_modules/" + mod.Module + "/.*" - if mod.Filename != "" { - path = "node_modules/" + mod.Module + "/" + mod.Filename + ".*" - } - build.OnLoad(api.OnLoadOptions{Filter: path, Namespace: "file"}, func(args api.OnLoadArgs) (api.OnLoadResult, error) { + filter := "node_modules/" + mod.Module + "/.*" + if mod.Filename != "" { + filter = "node_modules/" + mod.Module + "/" + mod.Filename + ".*" + } + build.OnLoad(api.OnLoadOptions{Filter: filter, Namespace: "file"}, func(args api.OnLoadArgs) (api.OnLoadResult, error) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/bundler/patch.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Analyze (go)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation