peggyjs / peggy

Peggy: Parser generator for JavaScript
https://peggyjs.org/
MIT License
914 stars 65 forks source link

Type of `ParserBuildOptions` is wrong #403

Closed siefkenj closed 1 year ago

siefkenj commented 1 year ago

In

https://github.com/peggyjs/peggy/blob/dfca04cf000d9be3d8f4a99314ccbedef744db33/lib/peg.d.ts#L1193

ParserBuildOptions["output"] is listed explicitly as "parser". I believe it should be SourceOutputs as defined a couple lines below.

hildjj commented 1 year ago

As it turns out, the type there is correct. See this test as an example. In this case, the output of generate changes based on the value of output. If I had it to do over again or didn't care about backward-compatibility, I think I would have made different method names that returned different types. As-is, the types are more complex to understand than is ideal.

siefkenj commented 1 year ago

I am confused. Is there somewhere else in the types file that allows output to be something other than "parser"? In the documentation line directly above line 1193 it says that several strings besides "parser" are permitted. TS also complained in my project when I passed output: "source" (I had to use a // @ts-ignore to suppress the error...)

hildjj commented 1 year ago

Yes. There are several overloads for generate, selected based on the different types of the options parameter. Each of those return something different based on what output you select.

export function generate(grammar: string, options?: ParserBuildOptions): Parser;
export function generate(grammar: string, options: SourceBuildOptions<"source">): string;
export function generate(grammar: string, options: SourceBuildOptions<"source-with-inline-map">): string;
export function generate(grammar: string, options: SourceBuildOptions<"source-and-map">): SourceNode;
export function generate(grammar: string, options: SourceBuildOptions<SourceOutputs>): SourceNode | string;
export function generate(grammar: string, options: SourceOptionsBase<"ast">): ast.Grammar;