serpapi / serpapi-javascript

Scrape and parse search engine results using SerpApi.

https://serpapi.com

MIT License

48 stars 4 forks source link

Support Node.js 7 and higher #11

Closed sebastianquek closed 1 year ago

sebastianquek commented 1 year ago

Tested with the following Node.js versions. These versions were installed using nvm (e.g. nvm install 7).

v7.10.1
v8.17.0
v9.11.2
v10.24.1
v11.15.0
v12.22.12
v13.14.0
v14.21.1
v15.14.0
v16.19.1
v17.9.1
v18.14.1
v19.7.0

Changes

Most of the changes are related to removing the use of modern node features or by relying on a polyfill instead.

Replace AbortSignal.timeout(timeout) with a setTimeout
Replace Object.fromEntries with a for-loop
Use fetch from cross-fetch@3.1.4 (rather than Node's in-built fetch)
Polyfill globalThis, URL, URLSearchParams using core-js-pure@3.28.0

Misc changes

Update examples to be more catered for different node versions
Remove tests in the npm library.

Related to #6

aciddjus commented 1 year ago

Can we add tests in GitHub actions for different Node versions?

sebastianquek commented 1 year ago

@aciddjus Yes, that’s possible. Given we’ve manually tested it in our pairing session, what do you think if we add automated tests in a separate PR?

aciddjus commented 1 year ago

Since tests would be directly related to what the PR is addressing let's have them here. Manually testing things is fine, but we should almost never rely just on that when pushing to production.

sebastianquek commented 1 year ago

I've modified the approach to handle polyfills. Now if the runtime does not have native implementations of the functions/classes (i.e. fetch, globalThis, URL, URLSearchParams), then the associated polyfill is dynamically imported and used. This creates a robust way to support runtimes that already have a native implementation.

It opens up the possibility of supporting new runtimes that do not support most Node.js APIs such as Vercel Edge functions and Cloudflare Workers: https://github.com/serpapi/serpapi-javascript/pull/12

I've also temporarily disabled the home depot tests as that API is failing.

zyc9012 commented 1 year ago

As we are not targeting browsers, we do not need to polyfill the missing APIs. The new commits by me have removed all the dependencies for the nodejs package. Built-in Node modules is not introducing new deps on the deno side either.

Before:

After: