`.buildPath` allows invalid arguments

[sloanesturz]

I have an application with heavy use of searchParams and I'm looking for a way to make it harder for developers to invoke buildPath incorrectly.

In the example below, you can see that it's very easy to call buildPath with the searchParameters as the first argument.

I think this is because the first argument to buildPath in this situation is Record<never, never> instead of Record<string, never>.

Any tips on either: (1) where to fix this in the library (2) how to build the ROUTES differently to get type safety without any upstream fix (3) a pointer on if I'm doing something completely wrong.

Thanks!

import { route, string } from 'react-router-typesafe-routes/dom';

const ROUTES = {
  WIDGETS: route('widgets', { searchParams: { order: string() } }),
};

// This is what I want to invoke:
ROUTES.WIDGETS.buildPath({}, { order: 'asc' }); // '/widgets?order=asc'

// This silently fails -- it passes the typescript tests
ROUTES.WIDGETS.buildPath({ order: 'asc' }); // '/widgets'

[fenok]

Hi!

This is indeed an issue that I don't know how to fix. As far as I remember, I couldn't make it so that the params argument rejects params when there are no params in pathname (without breaking something else).

The next major version will make it slightly better, because buildPath will accept a single argument (like { params: {...}, searchParams: {...} }), but even then you will be able to accidentally pass search params as params and get the same behavior. Also, I can't give an ETA on this version because I'm stuck on a TS performance issue I don't have the knowledge to fix 😓.

There may be a way to fix this in the userland by wrapping route and adding a version of buildPath that behaves as described above and also replaces {} with never/Record<string, never>, but I don't have time to test it right now.

[sloanesturz]

I found this implementation for Equals, I cannot take credit for it [0]

type Exact<A, B> = (<T>() => T extends A ? 1 : 0) extends <T>() => T extends B
  ? 1
  : 0
  ? A extends B
    ? B extends A
      ? unknown
      : never
    : never
  : never;

And I put together this alternative for a function buildPath

export const buildPath = <
  TPath extends string,
  TPathTypes,
  TSearchTypes,
  THash extends string[],
  TStateTypes,
>(
  r: Route<TPath, TPathTypes, TSearchTypes, THash, TStateTypes>,
  pathParams: Exact<TPathTypes, Record<never, never>> extends never
    ? InParams<TPath, TPathTypes>
    : Record<string, never>,
  searchParams?: InSearchParams<TSearchTypes>
): string =>
  r.buildPath(pathParams as InParams<TPath, TPathTypes>, searchParams);

And this mini test suite passes in my codebase:

import { route, string } from 'react-router-typesafe-routes/dom';
import { buildPath } from 'src/lib/routes';

const TEST_ROUTES = {
  EXAMPLE: route(
    'example',
    { searchParams: { q: string() } },
    { DETAILS: route(':id') }
  ),
};

describe('buildPath', () => {
  it('exercises types', () => {
    expect(buildPath(TEST_ROUTES.EXAMPLE, {})).toEqual('/example');
    expect(buildPath(TEST_ROUTES.EXAMPLE, {}, { q: 'my query' })).toEqual(
      '/example?q=my+query'
    );

    expect(buildPath(TEST_ROUTES.EXAMPLE.DETAILS, { id: '123' })).toEqual(
      '/example/123'
    );

    // @ts-expect-error - missing an id here.
    expect(() => buildPath(TEST_ROUTES.EXAMPLE.DETAILS, {}, {})).toThrowError();

    // @ts-expect-error - no id applies to this route.
    expect(buildPath(TEST_ROUTES.EXAMPLE, { id: '123' }, {}));
  });
});

Would you take a patch to this library along these lines? Essentially trying to special case the {} type in the pathParameters argument. Do you see a reason why this wouldn't work? I'm obviously not very familiar with the way your (very clever and helpful so far!) library works.

0. https://stackoverflow.com/questions/53807517/how-to-test-if-two-types-are-exactly-the-same

[fenok]

I gave it another shot today and it turned out to be a fairly easy fix. Looks like you nudged me in the right direction, thanks!

The only thing that bothers me a bit is that it's now impossible to define an object with some params and pass it to a route without them (as opposed to passing it to a route with fewer params). Though it's probably a very rare use case.

The fix has been published. Please reopen the issue if I missed something.