fenok / react-router-typesafe-routes

Comprehensive and extensible type-safe routes for React Router v6 with first-class support for nested routes and param validation.
MIT License
145 stars 3 forks source link

Params are not encoded #45

Closed andrejilderda closed 11 months ago

andrejilderda commented 11 months ago

Hi @fenok,

Thanks for your work on this library!

I found that when using route.buildPath the params are not encoded, resulting in invalid links when slashes are used. Is this intentional? I created a Sandbox to demonstrate the issue. https://codesandbox.io/p/sandbox/falling-firefly-n4kvsg

fenok commented 11 months ago

Hello and thank you for using the library!

This is indeed intentional for two reasons:

For now, you can use a custom type to add encoding:

// Only usable for pathname params
const encodedString: ParamType<string> = {
  getPlainParam(value) {
    // It's always guaranteed that value is not 'undefined' here.
    return encodeURIComponent(String(value));
  },
  getTypedParam(value) {
    // We could treat 'undefined' in a special way to distinguish absent and invalid params.
    if (typeof value !== 'string') {
      throw new Error('Expected string');
    }

    return decodeURIComponent(value);
  },
};

This is admittedly not ideal, since you really only want to alter the internal parser behavior. I'll think about adding an option for providing a custom parser to built-in types like string()

andrejilderda commented 11 months ago

@fenok I see, that makes sense. Thanks for providing a solution (which is much more elegant than what I came up with)!

dylan-oleria commented 3 months ago

Hello, @fenok,

Is there any reason for folks in consumer-land not to create their own parser that is essentially equivalent to the internal parser but adds encoding/decoding on the stringify/parse steps respectively? Doing so would be inline with an approach I've considered recently, but am curious if you're aware of any problems this may produce?

fenok commented 3 months ago

Hello!

Apart from what is mentioned in this thread, there should be no other issues.

I should mention that, as of now, it's impossible to fully address those issues in consumer-land, because a parser doesn't know which param and URL part it processes. So you'll have to either apply the custom parser only where you need it, or encode everything and ignore excessive encoding and potential issues with star params.

dylan-oleria commented 3 months ago

@fenok ah, I see.

In that case. In that case, how would you suggest going about needing to encode/decode JSON-like structures in searchParams? I see the above solution works for path params well, but your note about

// Only usable for pathname params

is catching my eye.

fenok commented 3 months ago

ParamType, SearchParamType, and StateParamType are low-level types that can be used if you need as much control as possible (for instance, in the example above, ParamType is used to create a type that is only usable in the pathname part of a URL).

You can also use the type() helper to create a type that is closer to built-in types. For instance, the example above can be rewritten like this:

const stringValidator: Validator<string> = (value) => {
    if (typeof value !== "string") {
        throw new Error("Expected string");
    }

    return value;
};

const stringParserWithEncoding: Parser<string> = {
    stringify(value) {
        return encodeURIComponent(value);
    },
    parse(value) {
        return decodeURIComponent(value);
    },
};

const encodedString = type(stringValidator, stringParserWithEncoding);

This type is usable anywhere and has modifiers like .default() and so on.

You can write parsers and validators for any JSONs you need, but if you have a lot of them, it's probably better to incorporate a validation library (please refer to Advanced Examples). It's also worth mentioning that built-in parser can handle JSONs (it simply uses JSON.stringify()/JSON.parse() for them).

TL;DR: