Repository navigation
Remove dead peechy schema scaffolding, bun-wasm, and write-only option types - #37068
Conversation
…n types The `.peechy` schema files and the `packages/bun-wasm` package are leftovers from an unshipped project: nothing generates code from the schemas anymore (the Rust and JS sides have both diverged from them), and bun-wasm has had no build path since the Makefile went away. - Delete `packages/bun-wasm/`, `src/api/schema.peechy`, `src/analytics/schema.peechy`, and the `src/api` npm scaffolding. - Inline the three tiny analytics enums into `bun_analytics` (dropping the never-constructed `None` variants) and reuse `bun_core::Environment::ARCH` instead of a duplicate `Architecture` enum. - Drop option types that were only ever written, never read: `ResolveMode` / `TransformOptions.resolve`, `CssInJsBehavior` / the printer's `css_import_behavior`, `LoadedEnvConfig` / `Env::to_api`, the never-populated `Env.defaults` list and the framework-defaults allowlist plumbing in `copy_env_for_define`, `NpmRegistry::dupe`, the `SourceMap`/`Packages` type aliases, and the empty `bun_core::schema` module. - Remove comments promising a future peechy codegen pass. No behavior change: `bun build --env`, `--define`, `--sourcemap`, `--packages`, `.npmrc` scoped registries, `/bun:info`, the dev-mode error page and the crash report platform line all produce the same output as before.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (30)
💤 Files with no reviewable changes (19)
WalkthroughThe PR removes the Bun WASM package and legacy schema files. It adds local analytics platform types, simplifies environment define handling, removes CSS printer configuration, and updates runtime consumers for renamed options and platform data. ChangesSchema and runtime cleanup
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
I didn't find any bugs, but this is a 32-file dead-code removal touching bundler options, CLI arg parsing, analytics, and the crash handler, so it's worth a human look.
What was reviewed:
- Traced the
copy_env_for_definerewrite:Env.defaultswas never populated (onlyEnvList::default()), so the removed framework-defaults path andany_prefix_matchgate were dead; the simplified loop is behavior-equivalent for bothPrefixandLoadAllmodes. - Verified
/bun:infoarch output is unchanged on shipped targets:bun_core::Environment::ARCHmaps x64→"x64"and aarch64→"arm", matching the oldPLATFORM_ARCHvalues. - Confirmed no remaining readers of
ResolveMode,CssInJsBehavior,css_import_behavior,api::SourceMap/api::Packagesaliases,bun_core::schema, orNpmRegistry::dupe(the ini caller now uses.clone(), whichdupealready delegated to). - Checked
for_os()still covers every supported target after the fallback arm was dropped; unsupported targets now fail at compile time instead of returningOperatingSystem::None.
Extended reasoning...
Overview
Dead-code removal across 32 files: deletes packages/bun-wasm/ (no build path since the Makefile was removed), the two .peechy schema files and their npm scaffolding, and prunes write-only option-type plumbing (ResolveMode, CssInJsBehavior, LoadedEnvConfig/Env::to_api, Env.defaults, NpmRegistry::dupe, the SourceMap/Packages type aliases, bun_core::schema). The analytics OperatingSystem/Platform types are inlined into bun_analytics and Platform.arch is dropped in favor of the compile-time bun_core::Environment::ARCH const.
Security risks
None. This is deletion of unused types, unbuildable packages, and comment cleanup. No parsing of untrusted input is added or changed; no auth/crypto/TLS paths are touched.
Level of scrutiny
Medium-high. Most of the diff is pure deletion (peechy files, bun-wasm, type aliases, comments), but two pieces need reasoning:
- The
copy_env_for_definerewrite insrc/bundler/defines.rscollapses ~60 lines to ~20 by removing the framework-defaults branch. I traced thatEnv.defaultswas only everEnvList::default()(never written to), soframework_defaults_keys/valueswere always empty at the sole call site, making every removed branch dead. Theany_prefix_matchpre-scan gate is also gone, but with empty defaults its only effect was skipping an already-no-op loop. - The analytics
Platformrefactor drops thearchfield and theNoneOS variant./bun:infonow readsbun_core::Environment::ARCH(compile-time const) instead of the runtime-populatedplatform.arch— output strings are identical on x64/aarch64.for_os()loses its catch-all arm, so an unsupported target now fails to compile rather than reporting_none.
Other factors
The PR description documents thorough manual verification (bun build --env=*, --define, --sourcemap, --packages, bun exec, bun run, .npmrc scoped registries, /bun:info, the dev error page, crash-report platform line) plus cross-target clippy for linux-gnu/musl/windows/freebsd/android. The bug hunting system found nothing. I'm deferring rather than approving because 32 files across bundler/CLI/analytics/crash-handler is enough surface area that a maintainer should confirm the "never populated" claims (particularly Env.defaults) match their understanding of the codebase history.
What does this PR do?
Deletes legacy code left over from the old peechy-schema /
bun-wasmeffort that never shipped. Nothing generates code from the.peechyfiles anymore — the Rust structs and the checked-in JS decoder have both diverged from them — andpackages/bun-wasmhas had no build path since the Makefile was removed, so all of it was dead weight that made the option types look like a wire format they no longer are.packages/bun-wasm/,src/api/schema.peechy,src/analytics/schema.peechy, and thesrc/apinpm scaffolding.bun_analyticsand reusesbun_core::Environment::ARCHinstead of a duplicateArchitectureenum.ResolveMode,CssInJsBehavior,LoadedEnvConfig/Env::to_api, the never-populatedEnv.defaultslist and the framework-defaults half ofcopy_env_for_define,NpmRegistry::dupe, theSourceMap/Packagesaliases, andbun_core::schema.No behavior change intended. Follow-ups (separate PRs): switch the dev-mode error page payload from the peechy binary encoding to JSON (which also lets
src/api/schema.jsand thepeechynpm dep go), and collapse theschema::apimirror enums (Loader/Target/SourceMapMode/…) onto their real counterparts.How did you verify your code works?
Debug build +
cargo clippy --workspace; cross-checkedbun_analytics/bun_crash_handlerfor linux-gnu, linux-musl, windows, freebsd and android targets. Drovebun-debugthrough each touched surface and confirmed identical output to before:bun build --env=inline|PUBLIC_*|disable,--define,--sourcemap,--packages, multi-entry--splitting,bun exec,bun run(shell),.npmrcscoped registry (request hit the scoped URL with its token),/bun:info, thedevelopment: trueerror page, and the crash-report platform line.test/bundler/bundler_env.test.tspasses.