Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Add node-fetch patch for isAbortSignal #433

Closed
jhaynie wants to merge 1 commit into from
Closed

Add node-fetch patch for isAbortSignal #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions internal/bundler/node-fetch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package bundler

func init() {
patches["node-fetch"] = patchModule{
Module: "node-fetch",
Filename: "src/utils/is",
Functions: map[string]patchAction{
"isAbortSignal": {
After: `if (result) { return true; }
if (_args[0] && _args[0].constructor.name === 'AbortSignal') {
return true;
}
`,
Comment on lines +9 to +13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider defensive programming for constructor name check.

The constructor.name property can be minified or altered in production builds, making this check potentially unreliable. Consider adding additional checks or using instanceof if the AbortSignal class is available.

After: `if (result) { return true; }
-if (_args[0] && _args[0].constructor.name === 'AbortSignal') {
+if (_args[0] && (_args[0].constructor.name === 'AbortSignal' || (typeof AbortSignal !== 'undefined' && _args[0] instanceof AbortSignal))) {
    return true;
}
`,
📝 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.

Suggested change
After: `if (result) { return true; }
if (_args[0] && _args[0].constructor.name === 'AbortSignal') {
return true;
}
`,
After: `if (result) { return true; }
if (_args[0] && (_args[0].constructor.name === 'AbortSignal' || (typeof AbortSignal !== 'undefined' && _args[0] instanceof AbortSignal))) {
return true;
}
`,
🤖 Prompt for AI Agents 
In internal/bundler/node-fetch.go around lines 9 to 13, the current check uses
_args[0].constructor.name === 'AbortSignal' which can break when names are
minified; replace it with a defensive sequence: first verify _args[0] is an
object, then if (typeof AbortSignal !== 'undefined' && _args[0] instanceof
AbortSignal) return true, otherwise fall back to duck-typing (e.g., check for
properties/methods like 'aborted' in _args[0] and typeof
_args[0].addEventListener === 'function') to reliably detect an AbortSignal
without relying on constructor.name.

},
},
}
}
29 changes: 25 additions & 4 deletions internal/bundler/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,26 +101,47 @@ func createPlugin(logger logger.Logger, dir string, shimSourceMap bool) api.Plug
}
contents := string(buf)
var suffix strings.Builder
isJS := strings.HasSuffix(args.Path, ".js")
for fn, mod := range mod.Functions {
fnname := "function " + fn
index := strings.Index(contents, fnname)
var isConstVariable bool
if index == -1 {
continue
fnname = "const " + fn + " = "
index = strings.Index(contents, fnname)
isConstVariable = true
if index == -1 {
continue
}
}
eol := searchBackwards(contents, index, '\n')
if eol < 0 {
continue
}
prefix := strings.TrimSpace(contents[eol+1 : index])
isAsync := strings.Contains(prefix, "async")
isExport := strings.Contains(prefix, "export")
newname := "__agentuity_" + fn
newfnname := "function " + newname
var newfnname string
if isConstVariable {
newfnname = "const " + newname + " = "
} else {
newfnname = "function " + newname
}
var fnprefix string
if isAsync {
fnprefix = "async "
}
if isExport {
fnprefix += "export " + fnprefix
Comment on lines +135 to +136
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect string concatenation in export prefix.

The export prefix is being concatenated incorrectly. The current code produces "export async " when it should produce "export async ". This results in duplicating the async keyword.

-if isExport {
-    fnprefix += "export " + fnprefix
-}
+if isExport {
+    fnprefix = "export " + fnprefix
+}
📝 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.

Suggested change
if isExport {
fnprefix += "export " + fnprefix
if isExport {
fnprefix = "export " + fnprefix
}
🤖 Prompt for AI Agents 
In internal/bundler/patch.go around lines 135 to 136, the code incorrectly
appends the current fnprefix to itself when adding the export keyword (fnprefix
+= "export " + fnprefix), causing duplication; replace that concatenation with a
simple prepend so that fnprefix is set to "export " + fnprefix (i.e., assign
rather than append) so the export prefix is added exactly once.

}
contents = strings.Replace(contents, fnname, newfnname, 1)
suffix.WriteString(fnprefix + fnname + "(...args) {\n")
if isJS {
suffix.WriteString(fnprefix + fnname + "() => {\n")
suffix.WriteString("let args = arguments;\n")
Comment on lines +139 to +141
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix malformed arrow function syntax for JS targets.

The generated arrow function syntax is incorrect. Arrow functions should be declared as const fnname = () => {}, not function fnname() => {}.

-if isJS {
-    suffix.WriteString(fnprefix + fnname + "() => {\n")
-    suffix.WriteString("let args = arguments;\n")
+if isJS {
+    suffix.WriteString(fnprefix + fnname + "(...args) => {\n")

Also note that arrow functions don't have their own arguments object. The args parameter should be used directly.

📝 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.

Suggested change
if isJS {
suffix.WriteString(fnprefix + fnname + "() => {\n")
suffix.WriteString("let args = arguments;\n")
if isJS {
suffix.WriteString(fnprefix + fnname + "(...args) => {\n")
}
🤖 Prompt for AI Agents 
In internal/bundler/patch.go around lines 139 to 141, the generated arrow
function syntax is malformed and incorrectly uses arguments; replace the current
output that writes fnprefix + fnname + "() => {" and the subsequent "let args =
arguments;" with a proper arrow function declaration that captures parameters,
e.g. write "const <fnname> = (...args) => {" for JS targets and remove any use
of the legacy arguments object so the code uses the args parameter directly.

} else {
suffix.WriteString(fnprefix + fnname + "(...args) {\n")
}
suffix.WriteString("\tlet _args = args;\n")
if mod.Before != "" {
suffix.WriteString(mod.Before)
Expand Down Expand Up @@ -150,7 +171,7 @@ func createPlugin(logger logger.Logger, dir string, shimSourceMap bool) api.Plug
}
}
loader := api.LoaderJS
if strings.HasSuffix(args.Path, ".ts") {
if !isJS {
loader = api.LoaderTS
}
return api.OnLoadResult{
Expand Down
Loading