feat: add ctx.renderNotFound to render custom _404 template

[xstevenyung]

Hello,

This PR will allow to render the _404.tsx if the handler return a 404 status code.

This is kind of mandatory when using dynamic routes where the data doesn't exists and a 404 has to be return from the handler function.

I filtered out to only invoke this logic when the Content-Type is either text/html or null (defaulting then to text/html) to avoid rendering HTML on application/json content type (for example on API endpoints).

I see that some discussion has already been made about this issue in https://github.com/denoland/fresh/issues/251

Feel free to close if you rather use a cleaner implementation.

[byhemechi]

This will fail if the Content-Type header includes the charset (e.g. Content-Type: text/html; charset=UTF-8).

It'd also probably be a good idea to write unit tests

[xstevenyung]

Good catch, I will add some testing

[xstevenyung]

added some tests and fix Content-Type issue.

thanks @byhemechi !

[lucacasonato]

I would rather do this differently. For example by allowing users to return null from handlers. Re-interpreting already created Response objects with 404 status is not something that will scale long term I think.

[xstevenyung]

@lucacasonato I guess I can see how this could be a problem long term. Rather than allowing to return null, do you think it would be better to add a function renderNotFound on the ctx of the handler to provide a more explicit way of invoking the _404 template ? If not, I'm fine with your suggestion, just trying to bounce out ideas.

[lucacasonato]

That works too

[byhemechi]

Another option would be to throw a response object for an error page?

[xstevenyung]

I like this idea but it might require more work to make it work. I'll go with the renderNotFound method and maybe something can be done later in another PR.

[xstevenyung]

It should be good to go, we now have a renderNotFound method with some tests and I also added some doc on the new method (would love to have feedback if the doc is clear enough).

There seems to be an issue with the CI on format check which is out of the scope of this PR.

[lucacasonato]

Another option would be to throw a response object for an error page?

Not super fond of this path, as throw is always very un-optimized in engines and should not really be used in a "hot path". This is also why I don't like how Reacts' suspense works :)

[xstevenyung]

Thanks for the documentation review, everything is merged 👍

[lucacasonato]

Can you rebase and resolve the conflicts?

[xstevenyung]

synced w/ main branch ✓