Improve query types #68
Conversation
| @@ -18,7 +18,7 @@ import { Contravariant1 } from 'fp-ts/lib/Contravariant' | |||
| * @category routes | |||
| * @since 0.4.0 | |||
| */ | |||
| export type QueryValues = string | Array<string> | undefined | |||
| export type QueryValues = string | Array<string> | |||
There was a problem hiding this comment.
@OliverJAsh this is a breaking change because the type is exported and we don't how and if it is used by the library's consumers.
Event if the version is still > 1.0.0, the package have been published from quite a lot by now and in my opinion this kind of changes has to be carefully evaluated.
Why do you propose it?
There was a problem hiding this comment.
Why do you propose it?
I think it was just to improve correctness. Consider this example:
declare const x: Query;
Object.entries(x).map(([, queryValues]) => {
// This will never be `undefined`, yet the type includes `undefined`.
queryValues
});There was a problem hiding this comment.
uhmmmm... I think that the QueryValues definition is a kind of "legacy" of Node's querystring package.
With the current implementation based on standard URLSearchParams it shouldn't happen to have an undefined query value.
Maybe we can add this change to a wider fp-ts-routing refactoring before publishing the v1.0.0
No description provided.