Repository navigation
node:string_decoder: construct subclasses of StringDecoder - #42582
Merged
Merged
Conversation
JSStringDecoderConstructor used one native function for [[Call]] and [[Construct]]. JSC passes new.target in the this slot of a native constructor, so `new Sub()` for `class Sub extends StringDecoder` reached the `StringDecoder.call(receiver, encoding)` arm with the class as the receiver. The arm wrote the decoder state onto the class and returned the class. Register separate call and construct functions. Both go through one createDecoder() body. For a [[Construct]] it takes the structure for new.target (InternalFunction::createSubclassStructure), so the instance has the prototype of the subclass. call() keeps the receiver arm for util.inherits-style constructors and is otherwise unchanged. One shared body keeps a single call site for the cell allocation, so the allocation still inlines. construct() compiles to a tail call into it. `new StringDecoder()` costs the same as before in a ThinLTO build. The Node.js compatibility page no longer lists subclassing as a gap.
Collaborator
Author
|
Updated 3:37 AM PT - Sep 13th, 2026
✅ @robobun, your commit 2717beff8e77e9b4e50bec74a5550d9c9866eb42 passed in 🧪 To try this PR locally: bunx bun-pr 42582That installs a local version of the PR into your bun-42582 --bun |
Collaborator
Author
|
Status
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. WalkthroughChangesStringDecoder subclassing
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to StringDecoder subclass construction and existing call behavior have no identified merge-blocking issue. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
LGTM — focused fix following the established getFunctionRealm → createSubclassStructure pattern, with thorough coverage.
What was reviewed:
- Exception handling in
createDecoder: every fallible call (getFunctionRealm,createSubclassStructure,parseEnumeration,toString,view) hasRETURN_IF_EXCEPTIONbefore use; the thinconstruct()wrapper encodesnullptr→ empty on the throw path, matching the sibling bindings. asObject(callFrame->newTarget())is safe — [[Construct]] guarantees an object newTarget; thecall()receiver branch is byte-for-byte preserved for iconv-lite.- Tests: combined-object
.toEqualshape,finally { delete Object.encoding }guards the unfixed-build side effect, revoked-Proxy and cross-realm cases exercise bothRETURN_IF_EXCEPTIONsites in the new branch.
Extended reasoning...
Overview
This PR splits JSStringDecoderConstructor's single native function into separate call() and construct() host functions so class Sub extends StringDecoder works. The shared createDecoder() helper resolves a subclass structure via InternalFunction::createSubclassStructure when newTarget differs from the base constructor, going through defaultGlobalObject(getFunctionRealm(...)) for cross-realm cases. Files touched: src/jsc/bindings/JSStringDecoder.{cpp,h} (~40 net lines), test/js/node/string_decoder/string-decoder.test.js (+110 lines of tests), and a one-line caveat removal from docs/runtime/nodejs-compat.mdx.
Security risks
None identified. This is prototype-chain plumbing for a Node compat constructor; no auth, crypto, filesystem, or network surface. The only user-observable side effect (reading newTarget.prototype, which can trigger a Proxy trap) is guarded by RETURN_IF_EXCEPTION and matches the ECMAScript-mandated ordering. No new allocation-size arithmetic or untrusted-length handling.
Level of scrutiny
Medium — native JSC binding code with exception-scope requirements. I checked each fallible call in createDecoder against REVIEW.md's "RETURN_IF_EXCEPTION before result is used" rule: getFunctionRealm (revoked Proxy throws — tested), createSubclassStructure (Proxy prototype trap — tested), parseEnumeration, toString, view, and ERR::UNKNOWN_ENCODING are all covered. The pattern is byte-for-byte identical to at least nine sibling bindings (JSDOMFile.cpp, NodeFSStatBinding.cpp, JSBakeResponse.cpp, JSHash.cpp, NodeDirent.cpp, etc.), so this is a mechanical application of the house idiom rather than a novel design. The construct() wrapper has no throw scope, but it's a pure tail-call whose nullptr result encodes to empty — the same shape JSC's own thin construct wrappers use, and the PR reports a clean BUN_JSC_validateExceptionChecks=1 run.
Other factors
Test coverage is strong: the entire pre-existing decode suite is now parametrized over a class extends subclass, and a new describe block covers Reflect.construct with foreign/Object/Proxy/revoked-Proxy newTarget plus node:vm cross-realm. The Object-as-newTarget test wraps assertions in try/finally { delete Object.encoding } so an unfixed build doesn't leak state into later tests — good hygiene per the "tests leave nothing behind" rule. Tests live in the existing module file, use combined-object .toEqual, and add vm as a static import. CODEOWNERS does not cover any changed path, there are no third-party reviews in the timeline, and the bug hunt ran to a dry streak with no findings. The PR description enumerates sibling sites sharing the root cause and explains why JSMockFunction is intentionally excluded (in-flight PRs), satisfying the "fix the whole class or say why not" rule.
usrbinkat
pushed a commit
to usrbinkat/bun
that referenced
this pull request
Sep 15, 2026
…2582) ### Problem - `class Sub extends StringDecoder {}; new Sub("utf8")` returns the class `Sub` itself. `s.write` is `undefined`, and `Sub` gains an own `encoding` property. `Reflect.construct(StringDecoder, ["utf8"], Object)` returns `Object`. Node returns an instance. - `JSStringDecoderConstructor` registers one native function for [[Call]] and [[Construct]] (`src/jsc/bindings/JSStringDecoder.h:118`). JSC passes new.target in the `this` slot of a native constructor, so the branch for `StringDecoder.call(receiver, encoding)` (`JSStringDecoder.cpp:592`) takes `Sub` as the receiver. - Found by an audit. No issue reports it. ### Fix - Register separate `call` and `construct` functions that share one `createDecoder` body. For a [[Construct]] it takes the structure from new.target with `InternalFunction::createSubclassStructure`, as `JSHash` and the generated classes do. `call` keeps the receiver branch unchanged. - The Node.js compatibility page no longer lists this gap. Not in this PR: `JSMockFunction.cpp:459` has the same cause, and oven-sh#34575 and oven-sh#41398 carry it. - `new StringDecoder()` costs the same as before (ThinLTO A/B in the notes). - Verified: `test/js/node/string_decoder/string-decoder.test.js` (143 pass, released bun fails the 45 new cases), Node's `test-string-decoder*.js`, `express-body-parser-test.test.ts`. Self-reviewed: 4 concerns raised, 4 addressed. ### Background - [[Call]] is `f()`. [[Construct]] is `new f()` or `Reflect.construct`. A JSC `InternalFunction` takes one native function for each. - new.target is the constructor that `new` was applied to: `Sub` for `new Sub()`. The new object takes its prototype from it. - A JSC `Structure` holds the prototype of an object. `createSubclassStructure` returns the base structure with the prototype of new.target. - The receiver branch serves iconv-lite 0.4 (under express body-parser), which runs `StringDecoder.call(this, enc)` on a plain object. It stores the native decoder on that object. <details><summary>Notes</summary> **Repro** ```js const { StringDecoder } = require("string_decoder"); class Sub extends StringDecoder {} const s = new Sub("utf8"); console.log(s === Sub, typeof s, s instanceof Sub, Reflect.ownKeys(Sub)); // bun 1.4.3: true function false [ "length", "name", "prototype", "encoding" ] // node v26.3.0: false object true [ 'length', 'name', 'prototype' ] // this branch: false object true [ "length", "name", "prototype" ] ``` **Sibling sites** (every place that registers one function for both [[Call]] and [[Construct]]) - `src/jsc/bindings/JSMockFunction.cpp:459`: `jsMockFunctionCall` reads `thisValue()`, so `new mockFn()` sees new.target as `this`. Same root cause. Excluded here because oven-sh#34575 and oven-sh#41398 already change that function. This PR does not fix `mock()` or `spyOn()`. - These never read `this` or new.target, so the confusion cannot happen there: the seven sink constructors from `src/codegen/generate-jssink.ts:59` (`js_construct` ignores the frame), `JSDOMConstructorNotConstructable` (throws on both paths), the `ObjectTemplate` `DummyCallback` in the V8 shim, `jsNodeVmWasmDisallowed` (`NodeVM.cpp:1046`, throws on both paths), `vm.createContext` (`NodeVM.cpp:1714`), and `SlowBuffer`. - oven-sh#42528 makes other native constructors honor new.target. It does not cover `StringDecoder`. **Cost of `new StringDecoder()`** A first version had a parse helper and called `JSStringDecoder::create` from both `call` and `construct`. With two call sites, `allocateCell<JSStringDecoder>` went out of line and `new StringDecoder()` got slower. One shared `createDecoder` keeps a single call site for the allocation, and `construct` compiles to 9 instructions that end in `jmp createDecoder`. ThinLTO release build, only this translation unit replaced, one pinned core, 300 reps of 200000 constructions, p50 in ns over 5 alternating rounds: | variant | `new StringDecoder()` | `new StringDecoder("utf8")` | | --- | --- | --- | | main | 11.6 to 12.3 | 19.5 to 21.1 | | first version | 14.9 to 16.1 | 19.8 to 20.9 | | this PR | 11.4 to 12.6 | 18.0 to 19.3 | **Behavior details** - Order matches Node: the `prototype` of new.target is read before the encoding is parsed (probed with a Proxy new.target and an encoding object with a `toString`). - `call` is unchanged, including the `thisValue != constructor` check, so `StringDecoder.call(StringDecoder, enc)` still returns a fresh decoder. The two `putDirect` lines in the receiver branch are textually untouched, so oven-sh#34609 and other work on that branch should merge without a conflict. - A new.target whose `prototype` is not an object gets `StringDecoder.prototype`, as for every JSC built-in class. Node gives `Object.prototype` there, because its `StringDecoder` is an ordinary function. - Cross-realm: a class defined in a `node:vm` context or a ShadowRealm can extend `StringDecoder`. The function realm of new.target is not always a `Zig::GlobalObject`, so the code goes through `defaultGlobalObject()`. A revoked Proxy as new.target throws a TypeError. - Remaining difference, not changed here: Node's `end(buf)` and `text()` call `this.write()`, so a subclass that overrides `write` sees those calls. Bun's native `end` and `text` call the native write directly. A plain instance with a patched `write` shows the same difference today. - Not changed here: `lastNeed`, `lastTotal` and `lastChar` throw on a `StringDecoder.call(this)` receiver, because the prototype accessors have a class check. Instances of a subclass are real `JSStringDecoder` cells, so the accessors work on them. **Suites run with the debug (ASAN) build** - `test/js/node/string_decoder/string-decoder.test.js`: 143 pass. The whole decode suite now also runs against `class SubStringDecoder extends StringDecoder`. Released bun 1.4.3: 98 pass, 45 fail. - The same file and a script of every throw path under `BUN_JSC_validateExceptionChecks=1`: clean. - `test/js/node/test/parallel/`: `test-string-decoder.js`, `test-string-decoder-end.js`, `test-string-decoder-fuzz.js`, `test-crypto-hash.js`, `test-fs-read-stream-encoding.js`, `test-readline-keys.js`, `test-stream-decoder-objectmode.js`, `test-stream-push-strings.js`, `test-stream-readable-setEncoding-existing-buffers.js`, `test-stream-readable-setEncoding-null.js`, `test-stream2-decode-partial.js`, `test-stream2-set-encoding.js`. - `test/js/third_party/body-parser/express-body-parser-test.test.ts`, and iconv-lite 0.4.24 (`StringDecoder.call(this, enc)` with `InternalDecoder.prototype = StringDecoder.prototype`) by hand. **Self-review** The review asked for four changes. All are in this PR: the stale docs line, the sibling-sites list, the slower `new StringDecoder()` of the first version, and multi-line comments in `src/`. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 4 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: BUILD FAILED (no junit output) $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/string_decoder/string-decoder.test.js ninja: Entering directory `/workspace/bun/build/debug' [1/164] cc obj/src/jsc/bindings/sqlite/sqlite3.c.o [2/164] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [3/164] gen ZigGeneratedClasses.{cpp,h,rs} Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts - ResolveMessage (15 fields) - BuildMessage (10 fields) Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts - Archive (4 fields, 1 class fields) Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts - ResourceUsage (8 fields) - Subprocess (20 fields) Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts - CronJob (5 fields) Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts - FileSystemRouter (5 fields) - FrameworkFileSystemRouter (2 fields) - MatchedRoute (8 fields) Found 1 classes from /workspace/bun/src/runtime/api/Glob.classes ... (truncated) release without fix: 45 FAILED bun test v1.4.3-canary.1 (6a92015) test/js/node/string_decoder/string-decoder.test.js: (pass) require('string_decoder') [0.10ms] (pass) Bun.inspect(StringDecoder) [0.09ms] (pass) FakeStringDecoderCall > StringDecoder-utf8 [1.88ms] (pass) FakeStringDecoderCall > StringDecoder-ucs-2 [0.94ms] (pass) FakeStringDecoderCall > StringDecoder-utf16le [0.04ms] (pass) FakeStringDecoderCall > StringDecoder-utf8-additional [0.11ms] (pass) FakeStringDecoderCall > StringDecoder-utf16le-additional [0.06ms] (pass) FakeStringDecoderCall > StringDecoder.end > base64 testbuf [0.18ms] (pass) FakeStringDecoderCall > StringDecoder.end > base64url testbuf [0.10ms] (pass) FakeStringDecoderCall > StringDecoder.end > hex testbuf [0.11ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf8 testbuf [0.06ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf16le testbuf [0.04ms] (pass) FakeStringDecoderCall > StringDecoder.end > ucs2 testbuf [0.05ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf8 partial "�a" [0.03ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf8 partial "��" (pass) FakeStringDecoderCall > StringDecoder.end > utf8 partial "��" (pass) FakeS ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/string_decoder/string-decoder.test.js bun test v1.4.3 (6a92015) test/js/node/string_decoder/string-decoder.test.js: (pass) require('string_decoder') [6.27ms] (pass) Bun.inspect(StringDecoder) [5.39ms] (pass) FakeStringDecoderCall > StringDecoder-utf8 [340.80ms] (pass) FakeStringDecoderCall > StringDecoder-ucs-2 [239.90ms] (pass) FakeStringDecoderCall > StringDecoder-utf16le [4.82ms] (pass) FakeStringDecoderCall > StringDecoder-utf8-additional [9.54ms] (pass) FakeStringDecoderCall > StringDecoder-utf16le-additional [8.09ms] (pass) FakeStringDecoderCall > StringDecoder.end > base64 testbuf [25.18ms] (pass) FakeStringDecoderCall > StringDecoder.end > base64url testbuf [17.47ms] (pass) FakeStringDecoderCall > StringDecoder.end > hex testbuf [13.70ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf8 testbuf [15.44ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf16le testbuf [11.48ms] (pass) FakeStringDecoderCall > StringDecoder.end > ucs2 testbuf [12.16ms] (pass) FakeStringDecoderCall > StringDecoder.end > utf ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 706ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/125] cc obj/src/jsc/bindings/sqlite/sqlite3.c.o [2/125] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [3/125] gen ZigGeneratedClasses.{cpp,h,rs} Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts - ResolveMessage (15 fields) - BuildMessage (10 fields) Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts - Archive (4 fields, 1 class fields) Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts - ResourceUsage (8 fields) - Subprocess (20 fields) Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts - CronJob (5 fields) Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts - FileSystemRouter (5 fields) - FrameworkFileSystemRouter (2 fields) - MatchedRoute (8 fields) Found 1 classes from /workspace/bun/src/runtime/api/Glob.classes.ts - Glob (5 fields) Found 1 classes from /workspace/bun/src/runtime/api ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` docs/runtime/nodejs-compat.mdx | 2 +- src/jsc/bindings/JSStringDecoder.cpp | 45 ++++++-- src/jsc/bindings/JSStringDecoder.h | 5 +- test/js/node/string_decoder/string-decoder.test.js | 114 ++++++++++++++++++++- 4 files changed, 151 insertions(+), 15 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests docs/runtime/nodejs-compat.mdx 1 1 18 src/jsc/bindings/JSStringDecoder.cpp 5 12 18 src/jsc/bindings/JSStringDecoder.h 1 1 18 test/js/node/string_decoder/string-decoder.test.js 2 5 18 ``` </details> <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
class Sub extends StringDecoder {}; new Sub("utf8")returns the classSubitself.s.writeisundefined, andSubgains an ownencodingproperty.Reflect.construct(StringDecoder, ["utf8"], Object)returnsObject. Node returns an instance.JSStringDecoderConstructorregisters one native function for [[Call]] and [[Construct]] (src/jsc/bindings/JSStringDecoder.h:118). JSC passes new.target in thethisslot of a native constructor, so the branch forStringDecoder.call(receiver, encoding)(JSStringDecoder.cpp:592) takesSubas the receiver.Fix
callandconstructfunctions that share onecreateDecoderbody. For a [[Construct]] it takes the structure from new.target withInternalFunction::createSubclassStructure, asJSHashand the generated classes do.callkeeps the receiver branch unchanged.JSMockFunction.cpp:459has the same cause, and Don't register native functions as their own constructors #34575 and bun:test: mock functions match Jest for Proxy receivers,new, and mock.instances #41398 carry it.new StringDecoder()costs the same as before (ThinLTO A/B in the notes).test/js/node/string_decoder/string-decoder.test.js(143 pass, released bun fails the 45 new cases), Node'stest-string-decoder*.js,express-body-parser-test.test.ts. Self-reviewed: 4 concerns raised, 4 addressed.Background
f(). [[Construct]] isnew f()orReflect.construct. A JSCInternalFunctiontakes one native function for each.newwas applied to:Subfornew Sub(). The new object takes its prototype from it.Structureholds the prototype of an object.createSubclassStructurereturns the base structure with the prototype of new.target.StringDecoder.call(this, enc)on a plain object. It stores the native decoder on that object.Notes
Repro
Sibling sites (every place that registers one function for both [[Call]] and [[Construct]])
src/jsc/bindings/JSMockFunction.cpp:459:jsMockFunctionCallreadsthisValue(), sonew mockFn()sees new.target asthis. Same root cause. Excluded here because Don't register native functions as their own constructors #34575 and bun:test: mock functions match Jest for Proxy receivers,new, and mock.instances #41398 already change that function. This PR does not fixmock()orspyOn().thisor new.target, so the confusion cannot happen there: the seven sink constructors fromsrc/codegen/generate-jssink.ts:59(js_constructignores the frame),JSDOMConstructorNotConstructable(throws on both paths), theObjectTemplateDummyCallbackin the V8 shim,jsNodeVmWasmDisallowed(NodeVM.cpp:1046, throws on both paths),vm.createContext(NodeVM.cpp:1714), andSlowBuffer.StringDecoder.Cost of
new StringDecoder()A first version had a parse helper and called
JSStringDecoder::createfrom bothcallandconstruct. With two call sites,allocateCell<JSStringDecoder>went out of line andnew StringDecoder()got slower. One sharedcreateDecoderkeeps a single call site for the allocation, andconstructcompiles to 9 instructions that end injmp createDecoder.ThinLTO release build, only this translation unit replaced, one pinned core, 300 reps of 200000 constructions, p50 in ns over 5 alternating rounds:
new StringDecoder()new StringDecoder("utf8")Behavior details
prototypeof new.target is read before the encoding is parsed (probed with a Proxy new.target and an encoding object with atoString).callis unchanged, including thethisValue != constructorcheck, soStringDecoder.call(StringDecoder, enc)still returns a fresh decoder. The twoputDirectlines in the receiver branch are textually untouched, so node:string_decoder: makeencodingan enumerable own data property #34609 and other work on that branch should merge without a conflict.prototypeis not an object getsStringDecoder.prototype, as for every JSC built-in class. Node givesObject.prototypethere, because itsStringDecoderis an ordinary function.node:vmcontext or a ShadowRealm can extendStringDecoder. The function realm of new.target is not always aZig::GlobalObject, so the code goes throughdefaultGlobalObject(). A revoked Proxy as new.target throws a TypeError.end(buf)andtext()callthis.write(), so a subclass that overrideswritesees those calls. Bun's nativeendandtextcall the native write directly. A plain instance with a patchedwriteshows the same difference today.lastNeed,lastTotalandlastCharthrow on aStringDecoder.call(this)receiver, because the prototype accessors have a class check. Instances of a subclass are realJSStringDecodercells, so the accessors work on them.Suites run with the debug (ASAN) build
test/js/node/string_decoder/string-decoder.test.js: 143 pass. The whole decode suite now also runs againstclass SubStringDecoder extends StringDecoder. Released bun 1.4.3: 98 pass, 45 fail.BUN_JSC_validateExceptionChecks=1: clean.test/js/node/test/parallel/:test-string-decoder.js,test-string-decoder-end.js,test-string-decoder-fuzz.js,test-crypto-hash.js,test-fs-read-stream-encoding.js,test-readline-keys.js,test-stream-decoder-objectmode.js,test-stream-push-strings.js,test-stream-readable-setEncoding-existing-buffers.js,test-stream-readable-setEncoding-null.js,test-stream2-decode-partial.js,test-stream2-set-encoding.js.test/js/third_party/body-parser/express-body-parser-test.test.ts, and iconv-lite 0.4.24 (StringDecoder.call(this, enc)withInternalDecoder.prototype = StringDecoder.prototype) by hand.Self-review
The review asked for four changes. All are in this PR: the stale docs line, the sibling-sites list, the slower
new StringDecoder()of the first version, and multi-line comments insrc/.[human-review] gate passed · iteration 0 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file