Skip to content

init: do not set type or private in a package.json that was loaded - #44992

Open
robobun wants to merge 4 commits into
mainfrom
robobun/271725ea/init-existing-type-private
Open

robobun wants to merge 4 commits into
mainfrom
robobun/271725ea/init-existing-type-private

Conversation

@robobun

@robobun robobun commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • bun init writes two new-project defaults into a package.json that it loaded: "type": "module" and "private": true. It replaces "commonjs" and false, adds both keys when absent, prints nothing, and exits 0.
  • Then node index.js fails with ReferenceError: require is not defined in ES module scope, and a Library-template package fails bun publish with error: attempted to publish a private package.
  • Cause: the two puts in InitCommand::exec (src/runtime/cli/init_command.rs:662-679) run for every manifest.

Fix

  • The two puts run only when bun init did not load a package.json (a new or empty file).
  • Correct because no other code writes these keys, and bun init adds its other fields only when absent.
  • Verified: test/cli/init/init.test.ts, 41 pass. Of 24 new or edited tests, main fails 21 and 3 are controls.
  • Self-reviewed: 35 concerns raised, 25 addressed, 10 moved with a split-off change (Notes).

Background

  • A loaded package.json is an existing one that parsed to an object. type decides how node loads .js files. private: true blocks publishing.
  • Considered a key-presence check: it still adds type to a CommonJS project with no type key. Keeping indentation and skipping a no-op write are separate follow-ups (Notes).

Downsides

  • Needs a maintainer's decision: a loaded package.json with no type no longer gains "type": "module", including the bun add and bun init -m stubs. One condition restores it.
  • Still open: bun init prints a loaded package.json again at 2 spaces, also with nothing to add.
  • Fresh bun init -y: +40 instructions in InitCommand::exec, +0 allocator calls, +0 syscalls. Release text: -768 bytes.
Notes

Where this comes from. A fuzz run of bun init over existing manifests found it. No user reported it. Related to #16361, which asks for a confirm step or --force when a package.json exists. This PR adds neither. type has been written since d034c00 (2022-08-04), private since #17282 (2025-02-18).

Repro (main 71d0d43. The report saw the same on 1.4.2 and 1.4.3):

D=$(mktemp -d); cd "$D"
printf '{"name":"cjs-app","version":"1.0.0","type":"commonjs","private":false}\n' > package.json
printf 'const os=require("os");console.log("require works:",os.platform());\n' > index.js
node index.js     # require works: linux
bun init -y
node index.js     # ReferenceError: require is not defined in ES module scope

On main the manifest ends with "type": "module" and "private": true. With this PR both values stay, and node index.js still works.

The decision. The two conditions are separate lines. The private condition also covers an absent key, because the Library template writes no private on purpose (fields.private = false) and a later bun init added it. The type condition is the open one:

  • With the condition (this PR): a CommonJS project with no type key keeps working under node. The stubs that bun add x and bun init -m write in an empty directory no longer end like a fresh bun init: they get no type and no private.
  • Without it (add type when absent): those stubs converge with a fresh bun init, and a typeless CommonJS project with a module field or no main still breaks under node.
  • A narrower rule from review: add type and private only when the loaded manifest has no type, main or module and the directory has no source file. The two stubs then end like a fresh bun init. A package that has only bin and keeps its CommonJS files in a subdirectory still breaks: with {"bin": {"cli": "./bin/cli.js"}}, node bin/cli.js goes from working to ReferenceError: require is not defined in ES module scope.

A matrix of 64 loaded manifests (type absent, "module", "commonjs", 123, times private absent, true, false, "yes", times no entry field, module, main, both), run with bun init -y on release builds:

value replaced type added private added
main 71d0d43 44 cells 12 cells 16 cells
this PR 0 0 0

Which manifests reach which path on main:

  • npm init -y (npm 11) writes main and "type": "commonjs". That manifest keeps its type and gains "private": true.
  • bun add ../dep in an empty directory writes {"dependencies": {...}}, and bun init -m -y writes {"devDependencies": {...}}. Both gain "type": "module" and "private": true. With this PR they gain name and module only.
  • A manifest with "type": "commonjs" and a module field or no main, or with "private": false, has the value replaced.

Tests. A new describe.concurrent block holds the offline tests. Every fixture lists @types/bun and typescript, so no nested bun install runs.

  • 16 rows: the type and entry values of the matrix above with private rotating. Each asserts the whole manifest.
  • The CommonJS manifest through -y, a stdin pipe, -y <folder> and a TTY.
  • The Library template's manifest (no private) and the bun add stub (no name).
  • In the old block: an empty package.json still gets the defaults of a new one, and the expected manifest of the non-object dependencies test loses type and private, because its fixture is a loaded manifest.

The 3 controls pass on main: type: "module" with private: true and no entry field, type: 123 with private: true and main, and the empty package.json.

Measurements (release builds, linux x64, main 71d0d43 against this PR):

  • InitCommand::exec 18,301 to 17,638 bytes, text 89,103,407 to 89,102,639 (llvm-nm, size).
  • Fresh bun init -y: package.json and stdout are byte-identical. Calls to has_property, as_property, put, put_string and print_json, allocator entry calls (56) and syscalls (149) do not change (gdb counters, 2 runs each).
  • Instructions from the entry of InitCommand::exec to its return on a fresh run: 69,741 to 69,781 (gdb stepi).
  • A package.json with nothing to add, in 9 layouts with -y and with -m -y: main and this PR both write 18 of 18, and 16 of 18 change bytes.

Not changed here. On a loaded manifest bun init still does these things:

  • It creates a console.log("Hello via Bun!") file at the entry path when nothing is at that exact path. For "main": "./index" beside index.js that file shadows the real module under node.
  • It adds "module": "index.ts" and the file when the directory has no source file, and the CommonJS manifest above gains "module": "index.js".
  • It adds @types/bun to devDependencies and typescript to peerDependencies when another dependency group already lists them, and then runs bun install.
  • It prints the file again at 2 spaces with a trailing newline, writes it when it has nothing to add, and prints no line about the edit.
  • A package.json that exists but does not parse is replaced by the scaffold (init: never write over a package.json that was not loaded #41807).
  • bun init --react returns before this code.

Open PRs on the same function. This PR adds 0 conflict regions against #41807, #41741, #35445, #44102, #38474, #39689, #36607 and #43489 (git merge-file on each head). #41807 removes did_load_package_json, so whichever lands second changes the two reads to its Loaded state. #43489 adds a test whose expected manifest has type and private for a loaded package.json, and it loses the two keys after this PR.

Self-review. The first version of this change also kept the indentation and the trailing-newline state of a loaded manifest, skipped the write when nothing changed, and printed ~ package.json. The review asked for a split: those parts conflict with three open PRs that rewrite the write block (#41807, #41741, #38474), the indentation part has two more sites (bun pm trust, and bun create in #36965), and the skipped write has no report behind it. That version is on the branch robobun/271725ea/init-keep-type-private (6881131). Four suggestions were not taken:

Windows was not run locally.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/init/init.test.ts

bun init over an existing package.json wrote "type": "module" whenever
the file had a "module" field or no "main", and "private": true on every
run without --minimal. Both writes replaced an existing value, and both
added the key when it was absent.

The two keys are now written only to a package.json that bun init did not
load: a new file or an empty one.
@robobun

robobun commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 6:55 AM PT - Oct 15th, 2026

✅ @robobun, your commit 0edeaefa97618b6060010b2e0c0a47bb1e3dc56f passed in Build #125905! 🎉


🧪   To try this PR locally:

bunx bun-pr 44992

That installs a local version of the PR into your bun-44992 executable, so you can run:

bun-44992 --bun

@robobun

robobun commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on main 71d0d43 (linux x64):

D=$(mktemp -d); cd "$D"
printf '{"name":"cjs-app","version":"1.0.0","type":"commonjs","private":false}\n' > package.json
printf 'const os=require("os");console.log("require works:",os.platform());\n' > index.js
node index.js     # require works: linux
bun init -y
node index.js     # ReferenceError: require is not defined in ES module scope

On main, package.json ends with "type": "module" and "private": true. With this PR both values stay and node index.js still works.

PR: #44992

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: aea5324a-2372-4800-a59e-b72f89022f01
📥 Commits

Reviewing files that changed from the base of the PR and between dfad7d0 and 0edeaef.

📒 Files selected for processing: 1
  • test/cli/init/init.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

bun init leaves existing package.json values for type and private unchanged. It still adds a module entry when neither main nor module exists. New projects receive the type: "module" and private: true defaults.

Changes

Manifest Defaults

Layer / File(s) Summary
Manifest defaults and existing-manifest coverage
src/runtime/cli/init_command.rs, test/cli/init/init.test.ts, docs/runtime/templating/init.mdx
bun init applies type and private defaults only when it did not load an existing manifest. Tests cover existing and empty manifests, initialization modes, and related manifest creation paths. Documentation describes the existing-manifest behavior.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 0edea

Existing package manifests retain their type and private settings, while new or empty manifests receive the defaults. No actionable merge risk was established; proceed with routine checks.

Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description clearly explains the problem, fix, verification results, scope, downsides, and remaining decisions. It does not use the template headings exactly, but it provides the required informat…
Title check Passed The title clearly and concisely describes the main change: preventing bun init from setting type or private in a loaded package.json.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread src/runtime/cli/init_command.rs Resolved
Comment thread test/cli/init/init.test.ts Resolved
Comment thread src/runtime/cli/init_command.rs Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment on lines +651 to +675
const decoder = new TextDecoder();
let output = "";
// `bun init` prints this note only when its stdin is a TTY.
const note = Promise.withResolvers<void>();
await using proc = Bun.spawn({
cmd: [bunExe(), "init"],
cwd: temp,
env: initEnv,
// A terminal that spawn creates reports EOF after the child exits and all of its output arrived.
terminal: {
cols: 80,
rows: 24,
data(_, chunk: Uint8Array) {
output += decoder.decode(chunk, { stream: true });
if (output.includes("package.json already exists")) note.resolve();
},
exit() {
note.reject(new Error(`bun init did not print its TTY note:\n${output}`));
},
},
});
const [exitCode] = await Promise.all([proc.exited, note.promise]);

expect(await Bun.file(path.join(temp, "package.json")).json()).toEqual(commonjsAfterInit);
expect(exitCode).toBe(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 nit (optional): the PTY test leaves its Bun.Terminal open after the child exits, unlike every other spawn-with-terminal test, so the PTY master fd leaks until GC. await using proc at test/cli/init/init.test.ts:651 does not close the terminal: Subprocess async dispose returns early once the child has exited (src/runtime/api/bun/subprocess.rs:619-621) and a terminal stays closed === false after exit. Fix: close the terminal on every exit path, e.g. proc.terminal!.close() in a try/finally after awaiting proc.exited, or await using terminal = proc.terminal! right after spawn, matching test/js/bun/terminal/terminal.test.ts:1299.

Why this was flagged

The test at test/cli/init/init.test.ts:642-676 spawns bun init with the terminal option and awaits proc.exited and note.promise at :675. It never calls proc.terminal.close(). await using proc runs Subprocess async dispose, which at src/runtime/api/bun/subprocess.rs:619-621 returns immediately when the child has already exited and leaves everything to GC, and the terminal is not closed by child exit (test/js/bun/terminal/terminal.test.ts:1298 asserts closed is false after exit). Every existing spawn-with-terminal test closes it explicitly (terminal.test.ts:1299, :1367, :1384, :1406, :1425). Under describe.concurrent on a long-lived CI runner the PTY master fd stays open until GC collects the wrapper; the base branch has no such test. Repo review guidance asks that every resource be released via using or try/finally registered before assertions.

Verification: Every run of the new PTY test at test/cli/init/init.test.ts:648-676 hits it: the child exits, nothing closes the terminal, and the master fd stays open until GC. await using proc (:655) runs Subprocess::async_dispose, which at src/runtime/api/bun/subprocess.rs:619-621 returns early once the child has exited. master_fd is only closed in close_internal (Terminal.rs:1745-1750).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants