Typing on parse; `parse` returns `Record<string, string | undefined>`

[RWOverdijk]

Hi,

I don't know if this is accurate to call a bug, so I'm filing it as a question 😄 If this is wrong, please let me know and I’ll change it.

I've been trying to figure out why the typing on parse defines a return type of Record<string, string | undefined>. From what I can tell, this seems to come from how ParseOptions is defined, specifically the decode function being typed as returning string | undefined. The default decode implementation always returns a string, so undefined doesn’t seem possible in normal usage.

My question is: what’s the reason for including undefined in the return type? Why not require ParseOptions.decode to return a string, or even better, make parse generic so its return type matches whatever the provided decoder returns? I might be missing a valid reason for the current typing.

The reason I noticed this is that it causes issues with code coverage. The typing forces me to handle the undefined case, but coverage then requires me to hit the “else” branch, which isn’t possible without either providing a custom decoder just to trigger it or using an istanbul ignore.

Assuming I’m not reading the code wrong, I’d propose making the typing more accurate and flexible. For example:

export type Decoder = (input: string) => unknown;
export type Cookies<V> = Record<string, V>;

export interface ParseOptions<Dec extends (str: string) => unknown = typeof decode> {
  decode?: Dec;
}

export function parseCookie<Dec extends Decoder = typeof decode>(
  str: string,
  options?: ParseOptions<Dec>,
): Cookies<ReturnType<Dec>>;

// Example 1: Override changes return type
const x1 = parseCookie("", {
  decode: (val: string): number => Number(val),
});

// Example 2: No override uses default return type
const x2 = parseCookie("");

// Example 3: Override that actually can return undefined
const x3 = parseCookie("", {
  decode: (val: string) => (val[0] === "a" ? val : undefined),
});

This way, the return type of parse is always linked to whatever the provided decode function actually returns, while keeping the default behavior as Record<string, string>.

Thanks for your time!

A screenshot from my IDE (have a giggle at my font) showing inference:

[blakeembrey]

what’s the reason for including undefined in the return type?

Mostly it's related to the decode function, as you noticed, allowing string | undefined. This is because undefined is explicitly handled as a way that could be used to skip over invalid values (current behavior matches 0.x behavior, but it is kind of weird that a badly encoded value gets "returned" instead of skipped).

Why not require ParseOptions.decode to return a string, or even better, make parse generic so its return type matches whatever the provided decoder returns?

Just complexity. If someone were to ask to encode/decode non-string values making it generic would certainly be the way to go here. IIRC it's not as easy as just providing a generic due to DX when providing an object, but I could be misremembering or TypeScript has improved this.

The reason I noticed this is that it causes issues with code coverage.

Since every key could by undefined anyway, I didn't expect this to increase consumption difficulty. Can you share an example of what it makes you do and I can look into improving this? I think we can probably get rid of the undefined on parse safely.

[RWOverdijk]

@blakeembrey Thanks for your response. I am using the example code I added in my issue now, and it works for me. It allowed me to remove the cast I had (as Record<string, string>). aUndefined is undefined and other values are other values. I also think it might be backwards compatible, since it wouldn't change for the default parse. Complexity is subjective but I find this approach pretty understandable.

Can you share an example of what it makes you do and I can look into improving this?

That is unfortunately difficult to do, but I Can summarise it as having to check if the value is set in the parsed cookies, even though we're looping based on the keys of the parsed cookies. So the value is always going to be there, and it is always going to be a string, but because of the undefined we need a condition where it is impossible to match the else case (without a custom parse).

To me this is minor, since simple workarounds like forking copy/pasting, casting or patching allow this to be solved in any code base quickly. Feel free to just close this issue if I am the only one or if you disagree 😄