Add ForeignRef<T> and make FetchHeaders the owned handle - #33820
Open
Jarred-Sumner wants to merge 2 commits into
Open
Add ForeignRef<T> and make FetchHeaders the owned handle#33820Jarred-Sumner wants to merge 2 commits into
Jarred-Sumner wants to merge 2 commits into
Claude / Claude Code Review
completed
Jul 10, 2026 in 28m 31s
Code review found 2 potential issues
Found 1 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/jsc/URL.rs:99-101 |
src/CLAUDE.md URL example is stale after URL becomes an owning handle |
Annotations
Check warning on line 101 in src/jsc/URL.rs
claude / Claude Code Review
src/CLAUDE.md URL example is stale after URL becomes an owning handle
This PR converts `bun_jsc::URL` into a `foreign_handle!` owning type — `URL::from_utf8` now returns `Option<URL>` (freed on `Drop`) and `URL::destroy` is deleted — but `src/CLAUDE.md` (lines 158–160) still documents the old `Option<NonNull<URL>>` return and shows `unsafe { URL::destroy(url.as_ptr()) }` for cleanup. Since the PR is what invalidates that example, it'd be worth refreshing those three lines here (drop the `destroy` line, note that the returned handle frees on drop). Doc-only.
Loading