keep query params on proxy

[dancastillo]

Checklist

closes: #322

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

Can you use https://www.npmjs.com/package/fast-querystring/v/1.1.0 instead?

[dancastillo]

@mcollina should be updated to use fast-querystring now

[gurgunday]

Looks good!

Since Matteo proposed changes to benefit performance, I think we can lower the amount of object creations and repetition

In line 337, dest === path because we initialize dest like so:

let dest = request.raw.url.slice(0, queryParamIndex !== -1 ? queryParamIndex : undefined)

We are essentially redoing this inside the function you've added using split, so I propose a small simplification where you call extractUrlComponents a little earlier, maybe right after where queryParamIndex is defined, and initialize dest with path

Finally, inside extractUrlComponents, we don't want to create an object if queryString doesn't exist. Instead, we can initialize with null and reassign if necessary

Here's what it'd look like

  function extractUrlComponents(urlString) {
    const [path, queryString] = urlString.split("?");
    const components = {
      path,
      queryParams: null,
    };

    if (queryString) {
      components.queryParams = qs.parse(queryString);
    }

    return components;
  }

  // ...
  // ...

    // delete -> const queryParamIndex = request.raw.url.indexOf("?");
    const { path, queryParams } = extractUrlComponents(request.url);
    let dest = path;

    // ...
    // ...

      // instead of checking queryParamIndex we check if queryParams is truthy
      if (queryParams) {
        dest += `?${qs.stringify(queryParams)}`;
      }

[dancastillo]

@gurgunday great feedback! this should be updated now

[gurgunday]

lgtm