denoland / denoland/std

Thoughts on the CSV ParseOptions type

Open
#6,739 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
3.6k
Forks
681
PR merge metrics
No merged PRs in 30d

Description

Hello, I don't know if it is intentional and what the gains are of the CSV module's options that are passed in through the `parse` function.

This is the function signature:

```ts
export function parse(
input: string,
options: T,
): ParseResult
```

I understand the reason for having it extend like above gives the possibility of the parser to extract the `columns` attribute and use that to make the resulting array "know" which keys are in object.

But the current implementation allows the `options` to be any record that extends the `ParseOptions` type, allowing developers to add extra options to the object that are then ignored if just one valid key is given, as in this example:

```ts
parse('a;b;c', { skipFirstRow: true, lacyQuotes: true })
```

This doesn't get caught.

This can become an issue when making typos as the TypeScript compiler doesn't catch any mistakes.

I suggest instead that we make the function signature something like this:

```ts
type ExcludeInvalidKeys = Record<
Exclude,
never
>

export function parse(
input: string,
options: ExcludeInvalidKeys,
): ParseResult
```

Which now correctly rejects invalid keys, but the error could be nicer. I just don't have the TypeScript-fu to do it. The error becomes:

```
type 'false' is not assignable to 'never'

The expected type comes from property 'lacyQuotes' which is declared here on type '{ readonly skipFirstRow: true; readonly lacyQuotes: true }' ...
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.