Remove dead code from react_compiler, the WebCore/IDL bindings, install_jsc, node-fallbacks, and misc crates - #39420
Conversation
|
Updated 2:29 AM PT - Aug 17th, 2026
✅ @robobun, your commit 2bb691956050f7762ca74985c695f2e00dafb639 passed in 🧪 To try this PR locally: bunx bun-pr 39420That installs a local version of the PR into your bun-39420 --bun |
There was a problem hiding this comment.
Beyond the two inline nits, I spot-checked the non-pure-deletion hunks: the JSX509Certificate m_infoAccess/computeInfoAccess(legacy) removal is safe — jsX509CertificateGetter_infoAccess in JSX509CertificatePrototype.cpp reads view().getInfoAccess() directly and never touched the lazy property; kRestorePreviousState and FSWatcher remain defined and used internally after their export-object entries were dropped; and SafeMap (unlike SafeWeakSet) still has importers and stays in internal/primordials.js.
Extended reasoning...
This is a 171-file, -4.4k-line dead-code sweep spanning react_compiler, the WebCore/IDL C++ bindings, ncrypto, install_jsc, node-fallbacks, uws_sys, and dozens of leaf Rust crates. The bug-hunting pass surfaced two nits (an orphaned JSDOMConvertScheduledAction.h and a stale test comment), neither with runtime impact. I additionally verified the one hunk that isn't a pure deletion — the JSX509Certificate::computeInfoAccess legacy-parameter collapse — against its remaining caller in JSX509CertificatePrototype.cpp, and confirmed the src/js export-object removals (FSWatcher, kRestorePreviousState, SafeWeakSet) leave the underlying definitions intact for their internal users. Given the breadth (Event bitfield layout, ncrypto move-assignment operators, node-fallbacks lockfile) a human pass is still appropriate; this note just records what was already checked so it isn't re-derived.
9d31ae2 to
09f7bc4
Compare
09f7bc4 to
4870a79
Compare
|
Addressed the review in 4870a79:
Also kept the 8-line |
…ll_jsc, node-fallbacks, and misc crates Net -4,406 lines. Every removed item had zero references across src/, scripts/, test/, packages/ and the regenerated codegen output, and the removal builds (bun bd, rust:check-all on all 11 targets, clippy). - react_compiler: the _exp variant of the derived-computations-in-effects validation (~1.1k lines) that the pipeline never invoked, plus the SymbolHost alias, HirBox and is_use_state_type. - C++ bindings: unused IDL typed-array conversions, DeferredPromise helpers, unused IDL wrapper types and the IDLDate converter files, NetworkLoadMetrics fields, ncrypto helpers, the JSWorkerOptions / JSMIMEBindings / JSDOMIterator.cpp files, and assorted single declarations. - install_jsc: dependency_jsc.rs and update_request_jsc.rs, whose only consumers were bun:internal-for-testing exports no test imports; the lsanDoLeakCheck export and its host function; install_types re-export modules nothing named. - node-fallbacks: a commented-out util.types block and dependencies / tsconfig paths nothing imports. codegen: the Ruby unified-source generator superseded by scripts/build/unified.ts. - Rust: dead pub items in bun_jsc, bun_runtime (including the wasi directory-iterator backend), bun_css, uws_sys and the leaf crates; dead uws C shims with their Rust declarations. - src/js: unused REPL primordials entries (and the SafeWeakSet export they were the last importer of) and unused export-object entries. test/internal/source-lints/dead-symbols-react-compiler-webcore-idl-misc.test.ts guards the removed symbols and files.
4870a79 to
2bb6919
Compare
|
Rebased onto current main (2bb6919, no conflicts, same diff). The binary-size annotation on the previous build compared this branch against canary #99935, which is #39326 (rust std built without the backtrace symbolizer); the branch predated that commit, so the ~0.5-0.8 MB per-target delta was the missing baseline change rather than anything in this diff. The one test failure (bun-lock.test.ts) passed on retry. IDLDate staying in IDLTypes.h for now is intentional, see the "Left out on purpose" list in the description and the reply on the thread. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs; the two earlier nits (pragma comment wording, JSDOMConvertScheduledAction.h) and the comment-cop flags have all been addressed, and the IDLDate deferral is called out in the description. Because it's a ~5k-line deletion across 171 files spanning the C++ JSC bindings, ncrypto, uws_sys shims, node-fallbacks lockfile, and a large react_compiler validation removal, a human look would still be worthwhile.
What was reviewed:
JSX509Certificatem_infoAccess/computeInfoAccess(legacy)removal — the prototype getter readsview().getInfoAccess()directly and both remaining callers passedlegacy=true, so the non-legacy branch was dead.internal/fs/watchandreadline/interfaceexport-object trims — importers only destructurewatch/createIgnoreMatcher;kRestorePreviousStateis still used internally.ncrypto.hmove-operator=removals andEvent.hbitfield removals — build-verified; no implicit users.ArrayBuffer::alloc_typed_array_uninitUint8Arrayarm — no callers instantiate that const-generic value.
Extended reasoning...
Overview
This PR removes ~4,900 lines of dead code across 171 files: the ~1.1k-line experimental validate_no_derived_computations_in_effects_exp pass in react_compiler, unused WebCore/IDL binding helpers (typed-array converter specializations, JSDOMPromiseDeferred resolve variants, NetworkLoadMetrics fields, JSWorkerOptions/JSMIMEBindings/JSDOMConvertDate files), install_jsc host functions whose only consumers were unused bun:internal-for-testing exports, ~400 lines of node-fallbacks package.json/lockfile entries and commented-out code, WebKit's Ruby unified-source generator, and a long tail of unused Rust items across bun_jsc, bun_css, uws_sys, zlib_sys, sha_hmac, windows_sys, and leaf crates. A new source-lint test guards against reintroduction.
Security risks
None identified. The removals in ncrypto, boringssl_sys, sha_hmac, and csrf are unused helpers (BignumPointer::isZero, EVPKeyCtxPointer::sign, deprecated raw hasher instantiations, never-constructed error variants) — not changes to any live security path. The JSX509Certificate::computeInfoAccess change collapses a two-branch function to its only-called branch; the prototype getter's behavior is unchanged.
Level of scrutiny
High, due to breadth. Each individual removal is mechanical and compiler-verified (Rust/C++ removals with remaining callers would fail to build; rust:check-all passed all 11 target triples for the platform-gated windows_sys/fs_events/zlib_sys hunks). JS/codegen removals were grepped and I spot-checked the internal/fs/watch, readline/interface, and repl/node-primordials export trims against their importers. The node-fallbacks lockfile prune is the least compiler-verifiable piece, but the description states build-fallbacks.ts externalizes builtin names and bundler_browser.test.ts passes.
Other factors
All prior review feedback on this PR has been addressed or explicitly deferred with rationale (IDLDate and JSDOMConvertScheduledAction.h are noted in "Left out on purpose"). The description is unusually thorough and the verification story (bun bd, rust:check-all, targeted test suites) is strong. However, at 171 files including C++ JSC bindings, ncrypto, uWS C++ shims, and a package lockfile, this exceeds the "simple, mechanical, or obvious" threshold for auto-approval — a maintainer should sign off on a sweep of this scale.
|
CI is green on the rebased head (2bb6919, build 99949: 179/179 jobs, the only red-then-green entries are retry-passed flakes in install/hot/napi tests this diff does not touch). Against the same canary baseline the binary is now 0 to 32 KB smaller on every target, which confirms the earlier +0.5 MB annotation was the missing #39326 baseline and not this diff. Ready for a maintainer. For a focused look, the only hunks that are not plain deletions are: the |
Net -4,406 lines (171 files, +505 / -4,911). Everything removed has zero references across
src/,scripts/,test/,packages/and the regeneratedbuild/debug/codegen/output, and the removal builds: removing a Rust or C++ definition that still had a caller fails to compile or link, so a green build is the reference check for those; JS, codegen and manifest removals were additionally grepped by name (includingbun:internal-for-testingconsumers undertest/).Most of these were first identified by the sweeps that were closed yesterday for merge conflicts (#37272, #37062, #38439, #37089, #38703). This PR re-applies the subset that still applies on current main, minus anything an open PR already deletes and minus small hunks in files that change daily (see "Left out" below), plus a few new finds.
react_compiler (-1,166)
validate_no_derived_computations_in_effects_expand its ~22 exclusive helpers/types (~1.1k lines invalidation/validate_no_derived_computations_in_effects.rs). The pipeline only ever calls the non-_expvalidation; the_expenv-config flag was parsed from fixture pragmas and read by nothing.react-compiler-fixtures.test.tsnow lists the two pragmas as ignored instead of handled; the fixture suite still passes.SymbolHost(back-compat alias ofHost;DESIGN.mdupdated),HirBox,is_use_state_type,default_true.C++ bindings (-1,350 across 64 files)
JSDOMConvertBufferSource.h: the IDL typed-array specializations andtoPossiblyShared*Arrayhelpers for every element type no binding converts (only the Uint8Array/ArrayBuffer views are used).JSDOMPromiseDeferred.h/.cpp:resolveWithJSValue,resolveWithNewlyCreated,resolveCallbackValueWithNewlyCreatedand friends.IDLTypes.h: theIDLUnsupportedTypefamily, the IDB / WebGL /ScheduledActionwrappers and stale forward declarations;JSDOMConvertDate.h/.cpp(the onlyIDLDateconverter) deleted along with its two includes. The 8-lineIDLDatestruct itself stays for now (see "Left out").JSDOMConvertScheduledAction.h, orphaned in the same way, is already deleted by Remove dead code from C++ webcore headers, src/js, and pretty_format #35775, so it is not touched here.NetworkLoadMetrics.h: WebKit networking-stack fields/accessors bun never reads.JSWorkerOptions.h/.cpp(JSWorker.cppbuildsWorkerOptionsby hand),JSMIMEBindings.h/.cpp(createMIMEBindinghad no callers; include dropped fromZigGlobalObject.cpp),JSDOMIterator.cpp(addValueIterableMethods).ncrypto.h/.cpp:peekError,BignumPointer::isZero,EVPKeyCtxPointer::sign, unused copy/moveoperator=overloads and anAsymmetricKeyEncodingConfigconstructor.JSDOMOperationReturningPromise.h(call*ReturningOwnPromise),JSDOMAttribute.h(setPassingPropertyName,setStatic),JSDOMGuardedObject(DoNotRegisterWithGlobalObjectTag),Event/EventTarget(resetBeforeDispatch, default-handled flags,isNode()),EventEmitter::{eventTypes,eventListeners},HTTPHeaderIdentifiers::identifierFor,JSURLPatternResultconvertDictionarystubs,PerformanceTiming::monotonicTimeToIntegerMilliseconds,ContextDestructionObserver::protectedScriptExecutionContext,expectedEnumerationValues<CryptoKeyUsage>,BufferSource::mutableData/toBufferSource,BunString__toInt32,BunString::utf8ByteLength, theRef<StringImpl>overload oftoCrossThreadShareable,normalWorld(),TextEncoding(const String&),JSBuffercreateBuffer/constructFromEncodingoverloads, theJSX509Certificatem_infoAccesslazy property and the non-legacy branch ofcomputeInfoAccess(the prototype getter reads the view directly; x509 tests pass),BakeAdditionsToGlobalObject::wrapComponent,jsFunction_lsanDoLeakCheck, and the deadBunObject+exports.hmacro entries.install_jsc / install_types / bun:internal-for-testing (-344)
install_jsc/dependency_jsc.rsandupdate_request_jsc.rsdeleted: their only consumers were thenpa/npmTagexports ofbun:internal-for-testing, which no test imports. Thedispatch_js2native.rsre-exports, thegenerate-js2native.tsfile-map entry and thelsanDoLeakCheckexport (tests useisASANEnabled) go with them.install_types/lib.rs: theExternalString/SlicedString/SemverStringre-export modules; nothing names those paths (everything imports the types frombun_semver). New in this PR.node-fallbacks (-400) and codegen (-392)
util.js: a ~270 line commented-oututil.typesblock.package.json/bun.lock/tsconfig.json: dependencies and path mappings nothing imports (esbuild,buffer,events,util,url,process,path-browserify,os-browserify,timers-browserify,tty-browserify,vm-browserify, ...).build-fallbacks.tsmarks every builtin name external, and the remaining sources only import the packages still listed;bun install --frozen-lockfilein the directory is a no-op andbundler_browser.test.tspasses.src/codegen/generate-unified-source-bundles.rb: WebKit's Ruby generator, superseded byscripts/build/unified.ts; nothing invokes it.Rust crates (-1,200 across bun_jsc, bun_runtime, css, uws_sys and leaf crates)
BuiltinName::get+BUILTIN_NAME_MAP,MarkedArrayBuffer::to_js, the__dangerouslySetPtrwrapperjs_class_module!emitted into every class,ErrorBuilder::new,TopExceptionScope::new,Task::new,JSCell::to_js,JSGlobalObject::{to_js,ref_,ctx},job::{on_js_thread,off_thread},AbortReasonimpl,JSPromisesettle helpers,VM::has_termination_request,UUID::ZERO,TagPayload::get.target_os = "wasi"directory-iterator backend indir_iterator.rs(no shipped target is wasi and thebun_sys::wasimodule it imports does not exist), theDisplayimpls inassert/myers_diff.rs, the non-unix stub and not-macos escapes infs_events.rs(the file is only compiled on macOS),ArrayBufferSink::to_js, unused re-exports innode.rs/api/bun/spawn.rs/ffi/mod.rs(withabi_typeformatters narrowed topub(crate)),Error::UnableToDecode,MyersDiff::Error::OutOfMemory. The first three are new in this PR.eql/to_css/parseforwarders whose callers all go through theCssEql/ToCss/Parsetrait impls (values/calc.rsand friends),generics::{implement_eql,parse},TokenList::parse_with_options,CssString::parse.uws_loop_defer,uws_res_clear_corked_socket,uws_ws_iterate_topicsanduws_h3_req_get_parameterC shims plus their Rust declarations and wrappers (Loop::{uncork,wake,next_tick,run}),AnyResponse::init,SocketGroup::is_empty,socket.rsgroup()accessors and theSocketTcp/SocketTlsaliases,Opcode::Close,WindowsLoop.windows_sysconstants and theirbun_sys::windowsre-exports,zlib/zlib_sysdeclarations (deflateInit_,inflateInit_, thegz*file API, legacy type aliases),sha_hmacdeprecated-API hashers (SHA512raw,RIPEMD160,MD5_SHA1,Blake2evp),wyhashHashIntimpls for u16/u64,libarchivecommented-out Zig-era callbacks,bun_alloc(AllocError::name,usable_size,BSSList::init),clap::Error::WriteFailed,csrferror variants,pe::Error::{InputIsSigned,InsufficientSpace},mdSetextheader,opaque_mut_nn,cares_sysAddrInfo_hints::is_empty,boringssl_sysconstants,errnoModere-exports,bounded_array::get,string::write::Result,SplitIterator::rest,OutOfRangeValueimpls,symbol::Map::init,sql_jscre-exports.src/js (-32)
internal/repl/node-primordials.js; with that goneSafeWeakSethad no importer, sointernal/primordials.jsstops exporting it (new in this PR). Unused export-object entries ininternal/fs/watch.tsandinternal/readline/interface.js.scripts (-9)
glob-sources.tssrc/*.cpattern (matched nothing sinceasan-config.cwas deleted), the write-onlyBUN_DEP_*defines indepVersionsHeader.ts, the unreadkqueueconfig field.Verification
bun bd(full debug build) passes.bun run rust:check-all: all 11 target triples ok (covers the windows/darwin-only removals inwindows_sys,sys/windows,zlib_sys/win32.rs,fs_events.rs,windows-shim).cargo fmt --check,cargo clippy --workspace, clang-format on every touched C++ file, prettier, andbun run lintare clean.test/internal/source-lints/(including the newdead-symbols-react-compiler-webcore-idl-misc.test.tsthat guards these symbols, anddead-code-escapesagainst the updateddead-code-escape-limits.json),react-compiler-fixtures.test.ts,bundler_browser.test.ts, css, cryptohasher/hash, node:assert, zlib, url, events, websocket, inspect and x509 tests pass.serve.test.tshas the same 4 failures as the unmodified release build in this container (IPv6 / root port range), nothing else.Left out on purpose (follow-up candidates)
h2_frame_parser.rs(~2.2k lines, still dead, Remove dead code from react_compiler and the node:http2 frame parser #37272's diff still applies cleanly): the file has had 15 commits in the last two weeks, so it is better landed on its own.getStackTraceForThrownValue(Error.prepareStackTrace: index source URLs by visible frame, not by JSC frame #37450),validateOneOf(node: render native validateOneOf errors like Node and type-check vm microtaskMode #38401), the redis error variants (redis: harden connection lifecycle (Handshake enum, promise-settlement fixes, -1251 LOC) #34829),URL::from_js(Fold CppWebSocketRef onto ForeignRef with a release marker #33889 / jsc: free owned FFI allocations when the post-call trap check returns Err #34577),schema::apire-exports (Delete the schema::api mirror types and bun_api; one loader numbering across Rust/C++/JS #37095),FsPath(Remove the undispatched LoaderHooks::resolve hook and lint hook tables for dead fields #39327),NodeJSFSNullimpl (node:fs: accessSync() returns undefined, promises.access() resolves to undefined, symlink callback gets null #38065), the deprecated selectorto_css(css: eliminate all unreachable!() via type-level hardening #33332).bindings.cpp,ZigGlobalObject.cpp,Blob.rs,streams.rs,BunObject.rs,BunProcess.cpp, theManifestLoad::LoadFromMemoryparameter acrosssrc/install, the watcherloaderparameter), and the#[no_mangle]statics (Zig_ErrorCode*,Bun__versions_*) nothing on the C++ side reads.IDLDateinIDLTypes.h: its only converter is deleted here, but the struct itself is left in place so this PR's file deletions stay independent of the header edits; removing the struct is a one-hunk follow-up once the converter files are gone.ReadWithoutLaunchmode (~110 lines, overlaps install(windows): store bin-link metadata in :bunx NTFS stream instead of .bunx sidecar #36200),node_quic_binding.rsconstants JS never destructures (~35 lines), the nativeNodeJSFS.unwatchFile/FSWatcher.hasRef/QuicSession.silentClose/QuicEndpoint.refbindings JS never calls, never-constructedbun_install::Errorvariants, and ~160 lines of$-declarations insrc/js/builtins.d.tswith no users.[review] gate passed · iteration 1 · 171 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file