jeroenpelgrims / vue-use-query-param

Use url query parameter like regular refs in Vue, in a typesafe way.
9 stars 1 forks source link

Please match the input and output types of serialize and deserialize in ParamSerializationConfig #16

Open Howard-Lam-UnitedVanning opened 3 months ago

Howard-Lam-UnitedVanning commented 3 months ago

It should be possible that the query value is not defined in the url rather than being an empty string.

/resource?query_a=&query_b=1 is different from /resource?query_b=1

https://github.com/jeroenpelgrims/vue-use-query-param/blob/83a10408e1272b1de82e4b4d7d7227647cfa25b2/src/converters/index.ts

type ParamSerializationConfig<TParam> = {
    serialize: (value: TParam | undefined) => LocationQueryValue | LocationQueryValue[];
    deserialize: (value: LocationQueryValue | LocationQueryValue[]) => TParam;
};

should be

type ParamSerializationConfig<TParam> = {
    serialize: (value: TParam) => LocationQueryValue | LocationQueryValue[] | undefined;
    deserialize: (value?: LocationQueryValue | LocationQueryValue[]) => TParam;
};
jeroenpelgrims commented 3 months ago

Hi Howard,

Can you explain a bit more how you're using the query parameters?
Because this is already possible by assigning undefined the query parameter, then the query parameter will be removed from the url.

Look at the first example here: https://stackblitz.com/~/github.com/jeroenpelgrims/vue-use-query-param-example

If we for example have the following code, and we're on the path /home:

const foo = useQueryParam("foo", stringParam());

We can set a value on the url:

foo.value = 'bar';
// the url will now be /home?foo=bar

We can set it to an empty string:

foo.value = '';
// the url will now be /home?foo=

Or we can REMOVE foo from the url:

foo.value = undefined;
// the url will now be /home

This is also the reason that ParamSerializationConfig::serialize MUST accept undefined as a value, otherwise we can't unset a query parameter.

Or are you by any chance trying to implement a custom ParamSerializationConfig?

Howard-Lam-UnitedVanning commented 3 months ago

Ok let me give you an example why the typing needs to change.

interface Foo {
   bar: string; //doesn't matter here
}
function useFoo() {
    function serialize(ref: Foo|undefined): LocationQueryValue|LocationQueryValue[]|undefined {
        //you can ignore implmenetation, just focus on the type
        return ref ? JSON.stringify(ref) : undefined
    };
    function deserialize(q: LocationQueryValue|LocationQueryValue[]|undefined): Foo|undefined {
        return q ? JSON.parse(`${q}`) : undefined;
    }
    return {
        serialize,
        deserialize
    }
}
useQueryParam('foo', {
    //Type '(ref: model) => LocationQueryValue | LocationQueryValue[] | undefined' is not assignable to type '(value: Foo | undefined) => LocationQueryValue | LocationQueryValue[]'
    //cannot serialize to undefined (which means I can't serialise it to not having anything in the query in the address)
    serialize: useFoo().serialize,
    //'(q: LocationQueryValue | LocationQueryValue[] | undefined) => Foo | undefined' is not assignable to type '(value?: LocationQueryValue | LocationQueryValue[] | undefined) => Foo'
    // because TParam is Foo only not Foo|undefined, therefore, undefined is not accepted as an output
    deserialize: useFoo().deserialize,
})
jeroenpelgrims commented 3 months ago

I think for this specific example you could just use the object converter.
How to use this you can see in the example code.

Note that I use the ParamSerializationConfig type in every converter.
Also note that I define the config as follows in the converters:

By defining the generic type as <type> | undefined, the serialize function will also accept undefined.
The same goes for the deserialize function, it will return <type> | undefined.

Currently to unset a variable on the url, null should be returned from the serialize function.


The only downside that I discovered while inspecting this issue is that currently no distinction is being made between null and undefined.
Setting a ref to undefined or null both unsets the query variable on the url.
Maybe this is the root of your question? That you want to be able to distinguish between these 2 when (de)serializing?

This will have an impact on a bunch of other code as well though, and possibly warrant a major version bump. So I'm reluctant to make a change like that based on just one issue.

Howard-Lam-UnitedVanning commented 3 months ago

Like I mentioned in the code, the implementation is just placeholder, it's the typing that's problematic. I am not using the object converter to demonstrate clearly the type problem. My own use case of course is more complicated.

I believe what is being serialised should match what comes out of deserialisation and vice versa. If you're serialising undefined, it should be in the output of deserialise as well. And if you are allowed to deserialising undefined, the serialise function should be able to output undefined.

"currently no distinction is being made between null and undefined." Nulls don't matter here because LocationQueryValue | LocationQueryValue[] | undefined does not include null values. The problem here is you can deserialise "LocationQueryValue" or array of "LocationQueryValue" or "undefined" but serialise can't output "undefined" by your type definition therefore it can never be empty in the url going through the code. Even undefined must be serialised as either LocationQueryValue or LocationQueryValue[].

Howard-Lam-UnitedVanning commented 3 months ago

I have updated the title to better reflect the issue.