Skip to content

fix(deprecated): correct v1 signatures in the compat shim#1492

Merged
pi0 merged 1 commit into
h3js:mainfrom
n0liu:fix/deprecated-v1-signatures
Jul 22, 2026
Merged

fix(deprecated): correct v1 signatures in the compat shim#1492
pi0 merged 1 commit into
h3js:mainfrom
n0liu:fix/deprecated-v1-signatures

Conversation

@n0liu

@n0liu n0liu commented Jul 21, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Addresses two of the deprecated-typing items from #1482.

appendResponseHeaders (and its appendHeaders alias) declares headers: string, but the body iterates the argument with Object.entries. As typed the export is unusable: passing the record it actually wants is a type error, and passing the declared string yields index-keyed garbage headers. Its sibling setResponseHeaders/setHeaders already takes Record<string, string>, so this brings the append pair in line with it.

sendRedirect required code, but v1 defaulted to 302 and the delegated redirect() still has status = 302. sendNoContent directly above it already declares code?: number, so the two are consistent again.

Both changes are type-only — runtime behaviour is untouched and there is no bundle impact.

Type tests are added to test/unit/types.test-d.ts. They fail on main (TS2345 on the record argument, twice, and TS2554 for the missing code) and pass with the change. Locally vitest --run reports 55 files / 1561 passed with Type Errors no errors, and oxlint plus oxfmt --check are clean.

Summary by CodeRabbit

  • Bug Fixes
    • Updated deprecated redirect handling to allow the status code to be omitted.
    • Improved deprecated header helpers to accept multiple headers as key-value pairs.
    • Updated type definitions and validation to reflect the revised deprecated API signatures.

@n0liu
fix(deprecated): correct v1 signatures in the compat shim
516480d 
`appendResponseHeaders`/`appendHeaders` declared `headers: string` while the
body iterates it with `Object.entries`, so the exports were uncallable as
typed. `sendRedirect` required `code` even though v1 defaulted to 302 and the
delegated `redirect()` still does. Both now match their sibling exports
(`setResponseHeaders`/`setHeaders`, `sendNoContent`).
@n0liu
n0liu requested a review from pi0 as a code owner July 21, 2026 02:56
@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a95e53e-1823-47f7-bff4-016c81655b4e

📥 Commits

Reviewing files that changed from the base of the PR and between 0a9d5f9 and 516480d.

📒 Files selected for processing (2)
  • src/_deprecated.ts
  • test/unit/types.test-d.ts
📝 Walkthrough

Walkthrough

Deprecated redirect and response-header signatures were updated in _deprecated.ts, with type-level tests covering optional redirect codes and record-based headers.

Changes

Deprecated API signature updates

Layer / File(s) Summary
Update deprecated signatures and type tests
src/_deprecated.ts, test/unit/types.test-d.ts		 sendRedirect now accepts an optional status code; appendResponseHeaders and appendHeaders accept header records, with matching type assertions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: pi0

Poem

I’m a bunny with typings,
Hopping through records so neat,
Redirect codes may now be skipped,
Deprecated calls stay sweet.
Tests bloom like clover beneath my feet.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: fixing deprecated v1 compatibility-shim TypeScript signatures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@pi0
pi0 merged commit ae4d6ee into h3js:main Jul 22, 2026
7 checks passed
@n0liu

n0liu commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the quick review and merge! I'll pick up more items from #1482.

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

Reviewers

@pi0 pi0 pi0 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@n0liu @pi0