Skip to content

fix(csv-parse): type the delimiter_auto score callback arguments - #499

Open
dylanpulver wants to merge 1 commit into
adaltas:masterfrom
dylanpulver:fix/csv-parse-delimiter-auto-score-type
Open

fix(csv-parse): type the delimiter_auto score callback arguments#499
dylanpulver wants to merge 1 commit into
adaltas:masterfrom
dylanpulver:fix/csv-parse-delimiter-auto-score-type

Conversation

@dylanpulver

Copy link
Copy Markdown

OptionDelimiterAuto.score is declared as () => number, so a custom scoring function cannot be given the two arguments the parser passes it.

parse(input, {
  delimiter_auto: {
    score: (info, options) =>
      (info.total - info.std) * (options.preferred[info.char_code] || 1),
  },
});

Actual, from npx tsc --noEmit:

error TS2769: No overload matches this call.
    Type '(info: any, options: any) => number' is not assignable to type '() => number'.
      Target signature provides too few arguments. Expected 2 or more, but got 0.

Expected: it compiles. utils/delimiter_discover.js:46 calls options.score(info, options), and the default at api/normalize_options.js:237 has exactly that shape. The snippet above is a copy of that default, so writing a custom score in TypeScript needs a cast.

preferred and size went into the interface in the same commit as score (4f69946) and both match the runtime. score is the member that does not.

This types the callback with a ScoringFunction alias sitting next to CastingFunction, plus an InfoDelimiterAuto interface for the five properties delimiter_discover sets before it calls score.

The test added to test/option.delimiter_auto.ts fails tsc --noEmit, which npm test runs ahead of mocha, when the type change is reverted. It also covers the runtime path: a custom score that ranks : above ; flips the discovered delimiter on a:b;c\nd:e;f, and no test passed a custom score before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant