serpapi / serpapi-javascript

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

More lenient TypeScript types #8

Closed sebastianquek closed 1 year ago

sebastianquek commented 1 year ago

When SerpApi adds support for a new engine or search parameter, the TypeScript types in this library are not immediately updated. For TypeScript devs, they will see warnings that the engine or search parameter does not exist.

This PR looks into making the types more lenient so no warnings appear, while ensuring autocomplete works and providing a way to maintain type safety.

Related to https://github.com/serpapi/serpapi-javascript/issues/5

Effects

Factor  Works?
Does not show warnings for arbitrary engines
Yes image
Does not show warnings for arbitrary search params
Yes (because of getJson’s P extends EngineParameters<E>) image
Autocomplete for engine
Yes
Autocomplete for search params
Yes
Type-checking for inline params
Yes image
Types for safety
Yes via EngineParamaters<"...">
sebastianquek commented 1 year ago

Thanks @zyc9012!

Should we update the examples as well because it's showing warning for arbitrary search params? (The warnings are not consistent with inline usage)

That warning is expected because params have explicitly been set to satisfy GoogleParameters. This provides a way for users to have full type safety. I can see why this could be confusing though. What do you think if we add an additional block in the example without the satisfies GoogleParameters?

If we add arbitrary params to the above example. The warning becomes confusing. (It says asd is not in GoogleParameters, but we are supposed to support arbitrary params)

Yea, that's unexpected behavior. Super strange. I've added a Record<string, unknown> to resolve this. The only warning that should appear is that the required params are missing.

--- a/src/serpapi.ts
+++ b/src/serpapi.ts
@@ -71,7 +71,9 @@ const SEARCH_ARCHIVE_PATH = `/searches`;
  */
 export async function getJson<
   E extends EngineName = EngineName,
-  P extends EngineParameters<E> = EngineParameters<E>,
+  P extends (EngineParameters<E> & Record<string, unknown>) = EngineParameters<
+    E
+  >,
 >(
   engine: E,
   parameters: P,

image

zyc9012 commented 1 year ago

That warning is expected because params have explicitly been set to satisfy GoogleParameters. This provides a way for users to have full type safety. I can see why this could be confusing though. What do you think if we add an additional block in the example without the satisfies GoogleParameters?

I think the idiomatic way may be use some type wrappers? Such as SomeTypeWrapperAllowingArbitraryParam<GoogleParameters>, and use that after satifies. Just a suggestion, you can decide.

sebastianquek commented 1 year ago

I think the idiomatic way may be use some type wrappers? Such as SomeTypeWrapperAllowingArbitraryParam, and use that after satifies. Just a suggestion, you can decide.

That's a great idea, didn't think of that. I've used it for getJson's parameter type too.