kwhitley / itty-router

A little router.
MIT License
1.76k stars 78 forks source link

Does `json()` support `headers` in options? #210

Closed dom96 closed 6 months ago

dom96 commented 9 months ago

Describe the Issue

As far as I can tell passing custom headers to json() doesn't work. Is this expected or a bug?

Example Router Code

        const headers = new Headers();
        headers.append("Set-Cookie", "blah=true");
        return json({success: true}, {
          headers: headers
        })
kwhitley commented 8 months ago

It does - just pass in headers in object-syntax!

json({ success: true }, {
  headers: {
    'set-cookie': 'blah=true'
  }
})

Should do the trick :)

kwhitley commented 8 months ago

See: https://github.com/kwhitley/itty-router/pull/213

Appx 20 bytes adds support for this, but cascades to not only createResponse(), but all the downstream helpers as well (json, text, etc), so my leaning is not worth the addition when the current implementation is simpler and lower-byte.

RonaldTreur commented 8 months ago

@kwhitley Just adding my voice here, as this issue cost me a few hours to debug yesterday 😅 It feels weird to me that you can use the nice and handy Headers class everywhere but not when using these helpers. It does not just break this consistency, but requiring object-notation feels like a step back for me as well.

If you persist, I would, at the very least, detect the headers are in an unsupported format and alert developers to this. Of course, I should have looked at the code right away, but I just could not believe that Headers would work everywhere but here.

edit: my current workaround is to not use these helpers and always create a new Response from scratch.

kwhitley commented 6 months ago

Folded on this one - #213 was merged and will be released shortly :)

Thanks for your patience!