feat(context): ResponseInit accepts generics StatusCode for status

[yusukebe]

From merging #3763, c.json() or c.text() can no longer have a contentless status code as the second argument.

In this PR, if a method such as c.json() has ResponseInit as its second argument, specifying contentless code for the status property will cause a type error.

This is the correct behavior, and I think this PR should be accepted. However, it causes minor performance degradation in TypeScript, so we need to be careful about it.

The author should do the following, if applicable

• Add tests
• Run tests
• bun run format:fix && bun run lint:fix to format the code
• Add TSDoc/JSDoc to document the code

[github-actions]

Bundle size check

	main (b1335f9)	#3770 (4525947)	+/-
Bundle Size (B)	18,772B	18,772B	0B
Bundle Size (KB)	18.33K	18.33K	0K

Compiler Diagnostics

	main (b1335f9)	#3770 (4525947)	+/-
Files	256	256	0
Lines	115,031	115,035	4
Identifiers	111,978	112,000	22
Symbols	243,490	243,518	28
Types	204,698	204,737	39
Instantiations	3,044,219	3,044,276	57
Memory used	419,317K	420,031K	714K
I/O read	0.03s	0.02s	-0.01s
I/O write	0s	0s	0s
Parse time	0.66s	0.7s	0.04s
Bind time	0.27s	0.33s	0.06s
Check time	5.5s	5.56s	0.06s
Emit time	0s	0s	0s
Total time	6.43s	6.59s	0.16s

---

Reported by octocov

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.72%. Comparing base (b1335f9) to head (17571ae).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #3770    +/-   ##
========================================
  Coverage   91.72%   91.72%            
========================================
  Files         159      159            
  Lines       10175    10175            
  Branches     2991     2847   -144     
========================================
  Hits         9333     9333            
  Misses        841      841            
  Partials        1        1            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yusukebe]

It has Response | ResponseInit because ResponseInit no longer satisfies the type of Response.

[yusukebe]

Hey @askorupskyy @EdamAme-x

What do you think of this PR?

[askorupskyy]

@yusukebe i like this PR & the code looks good to me. Is there any way to measure the TS performance degradation this can cause?

[EdamAme-x]

I think it is a good feature.
However, as you are concerned, types performance may be an issue.

[yusukebe]

Thanks for the comments!

You can refer to theCompiler Diagnostics on the comment, but it's difficult to measure the impact of real-world performance.

[yusukebe]

Hey, @m-shaka. What do you think of the change in TypeScript performance due to this PR?

[askorupskyy]

@yusukebe @EdamAme-x I think the subject of types is a part of a much greater discussion, but to put it simply I think less inference overall = better. I think the Hono Context as a concept makes it hard to make Hono scalable enough for the Typescript compiler. In this particular case, if each context method, such as .text() or .json() were to return a set of properties such as status, headers, content, this would be a much simpler and scalable fix

[askorupskyy]

That being said, I think this particular PR will not change things much in terms of TS performance, but we have to be careful overall about populating Hono's context with more TS.

As for the performance metrics I will need to run some more benchmarks to get more sample data

[yusukebe]

Speaking of this change, I think it is necessary to make ResponseInit behave in the same way as #3770. So I think it is okay to merge it, ignoring the performance issue.

As for the performance of TypeScript, it is ideal to optimize it as a separate issue.

[askorupskyy]

@yusukebe makes sense. From what I can say from the performance metrics above, the numbers we have to pay attention to most is # of Types and Check Time. In this PR the diff for those is minimal so I suggest we merge this.

[m-shaka]

Our test case doesn't use the second argument of json, so I reckon it's not a suitable one to check the impact of this change

hono/perf-measures/type-check/scripts/generate-app.ts

Lines 8 to 18 in b1335f9

	export const app = new Hono()`
	for (let i = 1; i <= count; i++) {
	routes += `
	.get('/route${i}/:id', (c) => {
	return c.json({
	ok: true
	})
	})`
	}
	return routes
	}

[m-shaka]

I added { status: 200 } to handlers of the test case on my machine and here are the results, which look ok.

// main
Files:              256
Lines:           115062
Identifiers:     112327
Symbols:         244896
Types:           206318
Instantiations: 3047508
Memory used:    429779K
I/O read:         0.02s
I/O write:        0.00s
Parse time:       0.33s
Bind time:        0.12s
Check time:       2.97s
Emit time:        0.00s
Total time:       3.42s

// this branch
Files:              256
Lines:           115035
Identifiers:     112200
Symbols:         245525
Types:           206794
Instantiations: 3048953
Memory used:    425269K
I/O read:         0.02s
I/O write:        0.00s
Parse time:       0.33s
Bind time:        0.13s
Check time:       3.04s
Emit time:        0.00s
Total time:       3.50s

Instantiations increased but just slightly

[yusukebe]

@askorupskyy @EdamAme-x @m-shaka Thank you!

Let's go with this. Merging now.