hono icon indicating copy to clipboard operation
hono copied to clipboard

Custom query parser

Open bompi88 opened this issue 1 year ago • 10 comments

What is the feature you are proposing?

I want to be able to switch out the default query parser with something like qs.

I am able to use a validator to "hack something together", but if I'm using @hono/zod-openapi that will not be able to use the output of that validator.

Something like the getPath method would be sufficient enough for my use case.

EDIT:

Adding more context to the ticket. I'm using a special query "syntax" which is supported by qs. For example I can do ?amount[lte]=300 which resolves to:

{
  amount: {
    lte: 300
  }
}

For comparison here is how you can change the query parser in express. Not that we should follow their approach because I think that one looks messy.

var express = require('express');
var qs = require('qs');
var app = express();

app.set('query parser', function (str) {
  return qs.parse(str, opts);
});

bompi88 avatar Nov 14 '24 14:11 bompi88

Hi @bompi88

I like the idea of using something like the getPath method. I will also consider this issue.

yusukebe avatar Nov 15 '24 08:11 yusukebe

It is unrelated to query parsing, but it would be interesting to allow users to specify a custom JSON serializer as a good example of using something like getPath.

yusukebe avatar Nov 15 '24 08:11 yusukebe

@yusukebe thanks! I've updated the ticket with the example from the https://github.com/honojs/hono/pull/3565 + example how it is done in for example express (please not get too inspired by that approach).

bompi88 avatar Nov 15 '24 08:11 bompi88

@yusukebe I know that there is currently an ongoing PR regarding the URLSearchParams that is kind of related (and not), but let me know if there anything I can facilitate regarding this ticket.

bompi88 avatar Nov 15 '24 08:11 bompi88

Hi @yusukebe, I wanted to follow up as I haven’t heard back from you yet. We’re highly interested in adding a way to override this functionality. Would you be open to accepting contributions on this topic?

bompi88 avatar Nov 26 '24 10:11 bompi88

@bompi88

Sorry for the late reply! I am not eager to proceed because the implementation would be complicated. Also, as I said before, I am concerned about the behavior depending on the parameters. If you make a PR, I will review it, but that does not ensure that it will be merged.

yusukebe avatar Nov 26 '24 14:11 yusukebe

@yusukebe We've started to look into how the code is structured, and we see what concerns you have. I hope that you enable a more plugin approach for the query parser in the future.

We have an ugly hack that we currently use to get around the problem using a middleware.

// middlewares.ts
import qs from 'qs';
import { createMiddleware } from 'hono/factory';
import { URL } from 'url';

export const queryParseMiddleware = createMiddleware((c, next) => {
  const query = qs.parse(new URL(c.req.url).search, { ignoreQueryPrefix: true });
  // eslint-disable-next-line no-param-reassign, @typescript-eslint/no-explicit-any
  c.req.queries = () => query as any;
  return next();
});

// utils.ts
import { createRoute as openApiCreateRoute } from '@hono/zod-openapi';

export const createRoute: typeof openApiCreateRoute = (config) =>
  openApiCreateRoute({
    ...config,
    middleware: config.middleware
      ? [...(Array.isArray(config.middleware) ? config.middleware : [config.middleware]), queryParseMiddleware]
      : queryParseMiddleware,
  });


// routes.ts
import { createRoute } from './utils';

export const list = createRoute({
  path: '/whatever',
  method: 'get',
  request: {
    query: z.object({
      value: z.object({ lte: z.number() })
    })
  },
  tags,
  responses: {
    [StatusCodes.OK]: jsonContent(z.any(), 'The list of whatever'),
  },
});

bompi88 avatar Nov 27 '24 10:11 bompi88

@bompi88

Setting a value for c.req.queries is only for c.req.valid(), so it does not replace the query parsing in HonoRequest. Therefore, since this middleware is only for validation, I have no motivation to make it a built-in middleware.

yusukebe avatar Nov 28 '24 00:11 yusukebe

@yusukebe I've been thinking, it does not make sense to me making a PR unless I know you're motivated by the changes. I also do not want to touch more code than necessary. So the part of qs I really care about is the ability to "group" certain query params. I'm not really interested in using qs in itself. I would imagine that it would behave the same way as it currently does in hono, only that it groups params if enabled. So, with grouping "turned on" (can be discussed if it should be a toggle or not):

# simple example
/?g[a]=1&g[b]=2&c=3
{
  g: {
    a: '1',
    b: '2'
  },
  c: '3'
}

# If for some reason someone provides a plain `g`
/?g[a]=1&g[b]=2&g=whatever
{
  g: [
    {
      a: '1',
      b: '2'
    },
    'whatever'
  }
}

And I dont think having more levels than that makes that much sense. And by enabling more levels would complicate things a lot.

Would this proposal align with your vision for Hono? Is it something you’d be interested in integrating into the core?

bompi88 avatar Dec 03 '24 09:12 bompi88

hey @bompi88! i'm a little late to the party, but i'm wondering whether using Zod's preprocess would meet your needs.

const ZQuerySchema = z.preprocess(
  (data: unknown) => { /** do your custom parsing */ },
  z.object({
    /** the nested schema */
  })
)

its a simple solve, though it can have some downstream consequences for schema composability. if all you want is to do some key manipulation though, that's what i'd recommend. i've used it for webhook requests that look like this:

type RequestBody = {
  json: string; // looking at you, slack
}

you should also check out a new community hono/openapi solution. it also supports Zod, but allows for more flexible/incremental adoption: https://github.com/rhinobase/hono-openapi

ambergristle avatar Feb 05 '25 11:02 ambergristle