feat: bump hono to v3.12.8, add customer err tests for compress middl…

[ariskemper]

No description provided.

[usualoma]

@ariskemper
Thanks very much.
It seems there was an error in the way I corrected the problem on honojs/hono#2080.
I will correct it later.
Please let me put this PR on hold until then.

[usualoma]

@ariskemper

Thank you for your patience. I have confirmed that the error is resolved in 3.12.8. I think it is a bug in node-server that the content-type does not have UTF-8.

diff --git a/package.json b/package.json
index 732dd32..c20eb7e 100644
--- a/package.json
+++ b/package.json
@@ -68,7 +68,7 @@
     "@types/supertest": "^2.0.12",
     "@whatwg-node/fetch": "^0.9.14",
     "eslint": "^8.55.0",
-    "hono": "3.12.7",
+    "hono": "^3.12.8",
     "jest": "^29.6.1",
     "np": "^7.7.0",
     "publint": "^0.1.16",
diff --git a/src/listener.ts b/src/listener.ts
index 1e02c5c..e8da26c 100644
--- a/src/listener.ts
+++ b/src/listener.ts
@@ -25,7 +25,7 @@ const handleResponseError = (e: unknown, outgoing: ServerResponse | Http2ServerR
     console.info('The user aborted a request.')
   } else {
     console.error(e)
-    if (!outgoing.headersSent) outgoing.writeHead(500, { 'Content-Type': 'text/plain' })
+    if (!outgoing.headersSent) outgoing.writeHead(500, { 'Content-Type': 'text/plain;charset=UTF-8' })
     outgoing.end(`Error: ${err.message}`)
     outgoing.destroy(err)
   }
diff --git a/test/server.test.ts b/test/server.test.ts
index 66e6b99..58a5899 100644
--- a/test/server.test.ts
+++ b/test/server.test.ts
@@ -73,8 +73,7 @@ describe('Basic', () => {
     expect(res.headers['content-type']).toEqual('text/plain;charset=UTF-8')
   })
 
-  // it fails with hono 3.12.7
-  it.skip('Should return 200 response - GET /ponyfill', async () => {
+  it('Should return 200 response - GET /ponyfill', async () => {
     const res = await request(server).get('/ponyfill')
     expect(res.status).toBe(200)
     expect(res.headers['content-type']).toMatch(/text\/plain/)
diff --git a/yarn.lock b/yarn.lock
index e201646..2b35c64 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2924,10 +2924,10 @@ hexoid@^1.0.0:
   resolved "https://registry.yarnpkg.com/hexoid/-/hexoid-1.0.0.tgz#ad10c6573fb907de23d9ec63a711267d9dc9bc18"
   integrity sha512-QFLV0taWQOZtvIRIAdBChesmogZrtuXvVWsFHZTk2SU+anspqZ2vMnoLg7IE1+Uk16N19APic1BuF8bC8c2m5g==
 
-hono@3.12.7:
-  version "3.12.7"
-  resolved "https://registry.yarnpkg.com/hono/-/hono-3.12.7.tgz#76be7d8a2f43ef29bba47e663b1adfba264a3281"
-  integrity sha512-jfyIoE8D5I1PGj0tXAGqpQ2miAWBBuHFjJsWjh0hbBVludQ8QO4lF6KTbPvjMi4venp8Q6DyMoJ51CwP6L5LNA==
+hono@^3.12.8:
+  version "3.12.8"
+  resolved "https://registry.yarnpkg.com/hono/-/hono-3.12.8.tgz#7c137aa6ac7bcd2aec3f55b9596d71e97081963a"
+  integrity sha512-vnOEIRdqsp4uHE/dkOBr9EYmTsR86sD/FyG2xhfAQzR9udDRglN1nuO7SGc/7U3HfSorc6PSCNGN6upnVtCmfg==
 
 hosted-git-info@^2.1.4:
   version "2.8.9"

[ariskemper]

@usualoma @yusukebe i refactored tests, fix UTF-8 issue and bump the version. Should be ready for merge.

[usualoma]

@ariskemper
Thank you very much!

Please remove the console.log statement.

Is there any reason why it should be 3.12.8 instead of ^3.12.8 on a dare? I think ^3.12.8 is better for me, because I think the cognitive load is high if only this part differs from the surroundings, although the difference is not so big since both are packages we manage ourselves.

[usualoma]

@ariskemper Thank you! LGTM!

[ariskemper]

@yusukebe could you create a new release including this PR?

[yusukebe]

@ariskemper

Okay! But please run yarn lint:fix && yarn format:fix.

[usualoma]

@ariskemper
Thanks again. Just sorry, but I would like to request the following.

• Changing the style of if statements is not considered a refactoring by us
  • This is a linter issue, so please don't change the if statement as a refactoring in PR
  • https://github.com/orgs/honojs/discussions/2108

[ariskemper]

@usualoma i have seen, that if, else was used in many places and many times could be written differently or simplified, but this change was more objective changing if, else to use ternary operator to write it in a more concise way. Some devs prefer to use if, else over ternary operator.
I agree that eslint should take over styling, but could convert it to ternary operator?

[usualoma]

@ariskemper
Thanks for the explanation! I am not familiar with eslint, but you are right, eslint may not do the conversion to ternary operator.

In any case, I don't think changing the style of if statements is something to be mixed into PR as a refactoring, since it is largely a matter of preference.

If you think such a rewrite in general would be useful, please create a separate PR for it and convince us by writing a rationale why you think it would be useful.

[yusukebe]

Ternary is useful, but surely we should also specify in Linter whether ternary should be used or not.

https://eslint.org/docs/latest/rules/no-ternary

We can do this in ESLint with the no-ternary option. Later on I will change the ESLint rules to give an error when using this Ternary.

[ariskemper]

@usualoma @yusukebe it actually shortens the code and remove identation, that's why i prefer it over single line block if, else statements.
Offcourse, we should have eslint rules to allow/disallow it, depends on agreed code guidelines. I do not like large PRs when refactor some codebase, because it takes time to review and it's better to improve code in smaller iterations, when refactoring does not intruduce breaking changes and keeps same API interface.

[usualoma]

ternary

The ternary itself I don't dislike; I prefer to use it when returning.

return condition ? a : b

However, I think it is bad style to assign within ternary.

k === 'set-cookie' ? cookies.push(v) : (res[k] = v)

But sorry, about 0885eab, I don't think it's right to mix it in a PR that says "feat: bump hono to v3.12.8, add customer err tests for compress middl..."

[ariskemper]

@usualoma could you explain more in detail, why do you think it should be used just when it is returning a value or storing it in a variable and why do you think using it this way is bad style? I agree that i should put it in a separate PR.

[yusukebe]

Instead of disallowing all ternaries, it might be better to disallow a nested ternary. This is indeed very bad for readability.

[usualoma]

@ariskemper
I will declare ahead of time that the following is my preference.

I think ternary should be used as an "expression (returning a value)". I don't think it should be used as a "statement" for side effects.

This is fine, since ternary is used as an "expression",

condition ? a : b

This is bad because it is a ternary used as a "statement".

k === 'set-cookie' ? cookies.push(v) : (res[k] = v)

But still, I don't think it is good that this discussion is taking place in a PR that says "feat: bump hono to v3.12.8, add customer err tests for compress middl..."

[usualoma]

Once the rules are set, I will follow them, but for my taste, I find this example to be fine.
#130 (comment)

As for ternary, I think it is good when used as an "expression" and bad when used as a "statement" with the main purpose of side effects. Personally, however, I think that in many cases it is "fine to write it either way" and it is often better not to dare to subject it to refactoring.

[usualoma]

This PR did an excellent job and led to fixing bugs in hono itself. Thanks to @ariskemper for creating it.
honojs/hono#2080

I think 0885eab should be revert and merged as I think it does more than the job this PR should do.

I think it is better to discuss elsewhere whether 0885eab is needed or not.

[yusukebe]

Thanks both.

Merging now.