fix(magic-string): throw TypeError for non-string content args (#8905)

IWANABETHATGUY · IWANABETHATGUY · commit dd77c527a786 · 2026-03-25T16:19:56.000Z
## Summary
- Add JS-side type validation wrappers for all MagicString methods that accept content (append, prepend, appendLeft, appendRight, prependLeft, prependRight, overwrite, update)
- Throws TypeError with messages matching the JS magic-string library instead of napi-rs's generic Error
- Un-skips 3 related "should throw when given non-string content" tests

## Test plan
- [x] just test-node magic-string/MagicString -- all 915 tests pass, 0 failed
- [x] just test-node magic-string/rolldown-magic-string -- all 33 JS tests pass
- [x] Un-skipped non-string content tests for append, overwrite, and update all pass
diff --git a/packages/rolldown/src/binding-magic-string.ts b/packages/rolldown/src/binding-magic-string.ts
@@ -11,6 +11,80 @@ Object.defineProperty(NativeBindingMagicString.prototype, 'isRolldownMagicString
   configurable: false,
 });
 
+// Validate content type to match JS magic-string behavior.
+// napi-rs throws a generic Error on type mismatch, but JS magic-string throws TypeError.
+function assertString(content: unknown, msg: string): asserts content is string {
+  if (typeof content !== 'string') throw new TypeError(msg);
+}
+
+// Save native method refs before overriding.
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativeAppend = NativeBindingMagicString.prototype.append;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativePrepend = NativeBindingMagicString.prototype.prepend;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativeAppendLeft = NativeBindingMagicString.prototype.appendLeft;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativeAppendRight = NativeBindingMagicString.prototype.appendRight;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativePrependLeft = NativeBindingMagicString.prototype.prependLeft;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativePrependRight = NativeBindingMagicString.prototype.prependRight;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativeOverwrite = NativeBindingMagicString.prototype.overwrite;
+// eslint-disable-next-line @typescript-eslint/unbound-method
+const nativeUpdate = NativeBindingMagicString.prototype.update;
+
+NativeBindingMagicString.prototype.append = function (content: any): any {
+  assertString(content, 'outro content must be a string');
+  return nativeAppend.call(this, content);
+};
+
+NativeBindingMagicString.prototype.prepend = function (content: any): any {
+  assertString(content, 'outro content must be a string');
+  return nativePrepend.call(this, content);
+};
+
+NativeBindingMagicString.prototype.appendLeft = function (index: any, content: any): any {
+  assertString(content, 'inserted content must be a string');
+  return nativeAppendLeft.call(this, index, content);
+};
+
+NativeBindingMagicString.prototype.appendRight = function (index: any, content: any): any {
+  assertString(content, 'inserted content must be a string');
+  return nativeAppendRight.call(this, index, content);
+};
+
+NativeBindingMagicString.prototype.prependLeft = function (index: any, content: any): any {
+  assertString(content, 'inserted content must be a string');
+  return nativePrependLeft.call(this, index, content);
+};
+
+NativeBindingMagicString.prototype.prependRight = function (index: any, content: any): any {
+  assertString(content, 'inserted content must be a string');
+  return nativePrependRight.call(this, index, content);
+};
+
+NativeBindingMagicString.prototype.overwrite = function (
+  start: any,
+  end: any,
+  content: any,
+  options?: any,
+): any {
+  assertString(content, 'replacement content must be a string');
+  return nativeOverwrite.call(this, start, end, content, options);
+};
+
+NativeBindingMagicString.prototype.update = function (
+  start: any,
+  end: any,
+  content: any,
+  options?: any,
+): any {
+  assertString(content, 'replacement content must be a string');
+  return nativeUpdate.call(this, start, end, content, options);
+};
+
 // Override replace/replaceAll to support RegExp patterns.
 // String patterns delegate to the native Rust implementation.
 // RegExp patterns delegate to native replaceRegex which uses the regress crate
diff --git a/packages/rolldown/tests/magic-string/MagicString.test.ts b/packages/rolldown/tests/magic-string/MagicString.test.ts
@@ -37,7 +37,7 @@ describe('MagicString', () => {
       assert.strictEqual(s.append('xyz'), s);
     });
 
-    it.skip('should throw when given non-string content', () => {
+    it('should throw when given non-string content', () => {
       const s = new MagicString('');
       assert.throws(() => s.append([]), TypeError);
     });
@@ -1005,7 +1005,7 @@ describe('MagicString', () => {
       );
     });
 
-    it.skip('should throw when given non-string content', () => {
+    it('should throw when given non-string content', () => {
       const s = new MagicString('');
       assert.throws(() => s.overwrite(0, 1, []), TypeError);
     });
@@ -1146,7 +1146,7 @@ describe('MagicString', () => {
       );
     });
 
-    it.skip('should throw when given non-string content', () => {
+    it('should throw when given non-string content', () => {
       const s = new MagicString('');
       assert.throws(() => s.update(0, 1, []), TypeError);
     });
diff --git a/packages/rolldown/tests/magic-string/download-tests.mjs b/packages/rolldown/tests/magic-string/download-tests.mjs
@@ -82,7 +82,7 @@ const SKIP_DESCRIBE_BLOCKS = [
 
 // Individual tests to skip (by partial match of test name)
 const SKIP_TESTS = [
-  'should throw when given non-string content', // error handling differs
+  // Note: 'should throw when given non-string content' now works via JS-side TypeError wrapper
   // Note: 'should throw' broad pattern removed — overlapping replacement error tests now pass
   // Remaining 'should throw' tests are covered by specific patterns (non-string content, negative indices)
   // options-specific skips
