qs
qs copied to clipboard
Feature> emptyValue options for parse & stringify
Addresses #223.
Behavior:
- When parsing and encountering empty values, replaces them with opt.emptyValue (default to '')
- When stringifying and encountering value === opt.emptyValue, outputs a key without a value
@timhwang21 is this still something you're interested in?
@ljharb sure! Sorry this fell off my radar as we ended up solving the problem we were facing on our end.
I suggest the following.
INPUT
OUTPUT
""
{}
"a"
{a: null}
"a="
{a: ""}
"a=null"
{a: "null"}
"a[0]" (arrayFormat: "indices")
{a: []}
"a[0]=" (arrayFormat: "indices")
{a: [""]}
"a[]" (arrayFormat: "brackets")
{a: []}
"a[]=" (arrayFormat: "brackets")
{a: [""]}
{a: undefined}
""
{a: null}
"a"
{a: ""}
"a="
{a: "null"}
"a=null"
I believe interpreting the presense of a key as its value being truthy is up to the users.
qs
should, by default, just focus on conveying what keys are “defined” and what values, if any, they hold.
qs
doesn’t know whether "a"
is boolean or not.
If how others interpret the stringified is concerned, why don’t you add “Do not convey (do omit) the presense of null
ed keys” and “Do not convey (do omit) the presense of empty arrays” options?
Just adding a note that I'm interested in this functionality too.
While I mostly agree with @issuefiler's desired behaviour (a
is null, a=
is an empty string, 'undefined' implies key-not-present, ...), I also agree with the suggestion that this should be an option via options.emptyValue
.
Given that truthy/falsy checks can be tricky and non-obvious to reason about, it's worth being careful to reduce impact to existing codebases (especially since there are many of them) when they upgrade qs
.
@ljharb after thinking about this some more, I'm liking the idea of strictEmptyHandling
(and skipEmpty
as well) you suggested. I'm proposing the following API:
Existing options:
-
strictNullHandling
-- Now no longer does anything by itself. When set totrue
, setsstrictEmptyHandling
to true and setsemptyValues
tonull
. -
skipNulls
-- Now no longer does anything by itself. When set totrue
, setsskipEmpty
to true and setsemptyValues
tonull
.
New options:
-
emptyValues
-- Defaults tonull
. Represents the valueqs
treats as "empty." -
strictEmptyHandling
-
parse
-- when set totrue
and given a value without a trailing=
, parses asemptyValue
instead of''
. -
stringify
-- when set totrue
and given a k-v pair with where value=== emptyValue
, returns a the key without an equals sign.
-
-
skipEmpty
- ~
parse
~ -- not implemented -
stringify
-- when set totrue
and stringifying a k-v pair where value=== emptyValue
, returns the empty string.
- ~
Edge cases:
- When stringifying, we cannot treat multiple values as empty. We can allow passing an array
emptyValues
tostringify
; however, this breaks the parallel options setup betweenstringify
andparse
, because multiple empty values don't make sense forparse
. The use case would be that when stringifying, we can do things like treat bothnull
and''
as empty. Alternatively, we can stay with the API of the current PR and keep the concept of empty and strict null entirely separate; however, I feel this makes the API harder to understand. -
strictNullHandling: true
orskipNulls: true
ANDemptyValue != null
. The conflict arises fromstrictNullHandling
wanting to setemptyValue
tonull
. We can either have this throw an error like my current PR does or have the user-specifiedemptyValue
overridenull
. I think the latter is reasonable but confusing.
Summary:
- If not passing any value to
emptyValue
(meaning the default ofnull
is used),strict{Null,Empty}Handling
andskip{Nulls,Empty}
are equivalent. - Existing code that doesn't specify the new options will see no change in behavior.
- I think we should choose one of the two options I proposed above for handling the need to serialize BOTH
null
an''
as an empty value. This seems like a very reasonable and desirable use case. I personally favoroptions.emptyValue: any
forparse
andoptions.emptyValues: any[]
forstringify
.
Please let me know what you think. Thanks!
@timhwang21 I like your new API suggestion. I like throwing an error when conceptually conflicting options are passed.
I'm not sure I understand the last bullet point in the "Summary", but if you're able to update the PR, maybe it will become clear to me :-)