Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow throwing responses (like Remix) and automatically wrap them in HttpException #2897

Closed
ajaishankar opened this issue Jun 3, 2024 · 7 comments
Labels
enhancement New feature or request.

Comments

@ajaishankar
Copy link

ajaishankar commented Jun 3, 2024

What is the feature you are proposing?

Having a conditional response like the following breaks type inference as a generic Response is inferred

app.get("/profile", (c) => {
  return c.var.user ? c.json(c.var.user) : c.redirect("/login");
});

Feel the documentation could be enhanced to avoid conditional responses and use HttpException instead

app.get("/profile", (c) => {
  if (!c.var.user) {
    throw new HTTPException(undefined, {
      res: c.redirect("/login"),
    });
  }
  return c.json(c.var.user);
});

Since we are throwing an exception for the redirect, the return type is inferred correctly for the happy path.

Even better maybe hono could support throwing responses similar to Remix?

https://remix.run/docs/en/main/utils/redirect
https://remix.run/docs/en/main/guides/not-found#how-to-send-a-404

app.get("/profile", (c) => {
  if (!c.var.user) {
    throw c.redirect("/login")
  }
  return c.json(c.var.user);
});

This can be achieved with a simple middleware, but feel this might be a good enhancement?

@ajaishankar ajaishankar added the enhancement New feature or request. label Jun 3, 2024
@mvares
Copy link
Contributor

mvares commented Jun 5, 2024

hey @yusukebe, could you take a look at that?

@yusukebe
Copy link
Member

yusukebe commented Jun 5, 2024

Hi @ajaishankar

One of the reasons why it occurs is the c.redirect() does not support the RPC. We can make it support it. How about like this?

const app = new Hono()

const flag = true

const routes = app.get('/profile', (c) => {
  if (!flag) {
    return c.redirect('/', 301)
  }
  return c.json({ name: 'abc' }, 200)
})

const client = hc<typeof routes>('/')

const res = await client.profile.$get()

if (res.status === 301) {
  console.log(res.headers.get('Location'))
} else {
  const data = await res.json()
  //    ^ { name: string }
}

@mvares
Copy link
Contributor

mvares commented Jun 5, 2024

Will Hono implement this soon?

@yusukebe
Copy link
Member

yusukebe commented Jun 5, 2024

@molives yes.

@ajaishankar Can you check #2908?

@ajaishankar
Copy link
Author

@yusukebe

Commented on the PR, looks good...

Redirect is a common enough use case and nice to have support for this in hono RPC.

For something like Remix thrown response I'd suggested very easy to accomplish with a simple middleware. Again Hono makes everything super simple! 🙂

const thrownResponseToHttpException = createMiddleware(async (c, next) => {
  try {
    await next();
  } catch (e) {
    throw e instanceof Response ? new HTTPException(undefined, { res: e }) : e;
  }
});

@yusukebe
Copy link
Member

Hi @ajaishankar

Is this issue solved?

@ajaishankar
Copy link
Author

Yes, no need to add this to Hono since it can be accomplished with a middleware..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request.
Projects
None yet
Development

No branches or pull requests

3 participants