serpapi / serpapi-javascript

Scrape and parse search engine results using SerpApi.
https://serpapi.com
MIT License
45 stars 4 forks source link

Search parameters as first argument #9

Closed sebastianquek closed 1 year ago

sebastianquek commented 1 year ago

Previously, I couldn't get autocomplete to work correctly for the other search params once engine was defined, i.e. { engine: "google", <autocomplete for other params> }. By setting engine as the first argument of getJson and getHtml instead, the type of search params (2nd arg) could be narrowed down to provide accurate autocomplete.

This PR explores having the search params as the first argument of getJson/getHtml while ensuring arbitrary engines/search params are accepted, autocomplete works and a way to have type safety is maintained.

Related to #5

Attempts

1st try

I re-looked into this issue and found that I needed another abstraction, EngineParameters:

export type EngineName = keyof EngineMap;
export type EngineParameters<
  E extends EngineName = EngineName,
> = {
  [K in E]: { engine: K } & EngineMap[K]["parameters"];
}[E];

...

export async function getJson<
  E extends EngineName = EngineName,
  P extends EngineParameters<E> = EngineParameters<E>,
>(
  parameters: P,
  callback?: (json: BaseResponse<E>) => void,
) { ... }
Factor Works?
Does not show warnings for arbitrary engines No (because EngineName must be keyof EngineMap)
Does not show warnings for arbitrary search params Yes (because of getJson’s P extends EngineParameters<E>)
Autocomplete for engine Yes
Autocomplete for search params Yes
Type-checking for inline params Yes
Types for safety Yes via EngineParameters<"...">

2nd try

The problem still remained that an arbitrary engine shows a warning. Based on this TypeScript issue comment, there’s a way to support autocomplete whilst allowing for any string. I’ve adopted the concept here:

type AnyEngineName = string & {};
export type EngineName = (keyof EngineMap) | AnyEngineName;

EngineParameters was also modified to use the above.

export type EngineParameters<
  E extends EngineName = EngineName,
> = {
  [K in E]:
    & { engine: K }
    & (K extends keyof EngineMap ? EngineMap[K]["parameters"]
      : BaseParameters & Record<string, unknown>);
}[E];

What this does is that for all known engines (those in keyof EngineMap), the params are taken from EngineMap, while for unknown engines, params is anything that includes properties from BaseParameters.

Factor Works?
Does not show warnings for arbitrary engines Yes (because of AnyEngineName)
Does not show warnings for arbitrary search params Yes (because of getJson’s P extends EngineParameters<E>)
Autocomplete for engine No
Autocomplete for search params Not quite (types are showing as unknown)
Type-checking for inline params No
Types for safety Yes via EngineParameters<"...">

However, this change causes autocomplete for engine and type-checking for inline params to not work. Also, search params autocompletions showed unknown as their type.

3rd try

To fix the issue with search params autocompletions not reflecting the correct type, removing the Record<string, unknown> in EngineParameters worked.

export type EngineParameters<
  E extends EngineName = EngineName,
> = {
  [K in E]:
    & { engine: K }
    & (K extends keyof EngineMap ? EngineMap[K]["parameters"] : BaseParameters);
}[E];
Factor Works?
Does not show warnings for arbitrary engines Yes (because of AnyEngineName)
Does not show warnings for arbitrary search params Yes (because of getJson’s P extends EngineParameters<E>)
Autocomplete for engine No
Autocomplete for search params Yes
Type-checking for inline params No
Types for safety Yes via EngineParameters<"...">

Conclusion

I think what we have in the 3rd try is good enough. Users can supply any arbitrary engine/search params without seeing warnings, while still being able to receive hints as to what search params are supported and their types. For users who want more type safety, they can use EngineParameters<"..."> which only allows the explicitly defined parameters.

I can’t seem to find a way to get autocomplete for engine to work while taking care of all other factors. I think it’s an acceptable tradeoff as it’s still better to not show warnings for arbitrary engines. Perhaps we can revisit this in future.

Demos:

Autocomplete for search params, accepts arbitrary engines/search params

ts-params-first-arg.webm

Using EngineParameters<...> for type safety

ts-params-satisfies.webm


This is a breaking change, I suggest we release it as 2.0.0. Note that this is based on the work in #8 which should be merged first as a 1.x.x version.

sebastianquek commented 1 year ago

Thanks @zyc9012, I've made the changes while maintaining autocomplete for both variants. There's not too much extra code which is good.

I tried function overloads, similar to how I did it previously, but the intellisense outputs aren't super helpful for users. I opted to use a union type for the ..args instead:

  ...args:
    | [parameters: P1, callback?: (json: BaseResponse<E>) => void]
    | [engine: string, parameters: P2, callback?: (json: BaseResponse<E>) => void]

I also made sure that engine is typed as a string rather than EngineName to ensure that parameters continue to have autocomplete, image while allowing arbitrary types (in this case start can be a string) image

Please take a look and let me know if there's anything amiss! Thanks :)