Add support for request options

[zyc9012]

Resolves #27

This PR was initially for supporting the proxy feature natively. Unfortunately, HttpsProxyAgent isn't supported in some of the older NodeJS versions that this library promises to support.

As a fallback, this PR allows users to pass their own http.RequestOptions so they can set the agent to be the proxy agent they want.

[Copilot]

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• deno.json: Language not supported

Comments suppressed due to low confidence (1)

src/utils.ts:66

• [nitpick] The shallow merge of requestOptions may not correctly combine nested objects (e.g., headers). Consider using a deep merge strategy if preserving or merging nested properties is desired.

return { ...base, ...config.requestOptions, ...(parameters.requestOptions as http.RequestOptions), };

[Copilot]

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (1)

• deno.json: Language not supported

Comments suppressed due to low confidence (1)

src/utils.ts:91

• Since setEncoding('utf8') is called, the 'data' event returns strings rather than Buffers. Consider changing the type annotation to 'string' or removing it.

resp.on("data", (chunk: Buffer) => {

[Copilot]

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (1)

• deno.json: Language not supported

Comments suppressed due to low confidence (2)

src/utils.ts:66

• [nitpick] Consider using a deep merge for the headers property if both config.requestOptions and parameters.requestOptions include headers. This would allow combining header objects instead of one entirely overriding the other.

return { ...config.requestOptions, ...(parameters.requestOptions as http.RequestOptions), ...basicOptions };

tests/utils_test.ts:136

• [nitpick] Consider refining the type conversion instead of using an 'as unknown as' cast for requestOptions. Updating the parameter type definitions could improve type safety.

const params = { q: "coffee", requestOptions: customOptions } as unknown as qs.ParsedUrlQueryInput;

[Ovi]

Thank you, @zyc9012! 👍
Great work.

[Ovi]

deno task test fails, whereas deno task test:ci passes, I assume this is because we are using https module and it's trying to hit localhost with secure protocol (https)

Suggested change

	const req = https.get(options, handleResponse).on("error", handleError);
	const protocolModule = options.protocol === "https" ? https : http;
	const req = protocolModule.get(options, handleResponse).on("error", handleError);

[Ovi]

Let's make this dynamic based on the environment.

[Ovi]

Let's include protocol here

Suggested change

	? {
	hostname: "localhost",
	port: 3000,
	}
	: {
	hostname: "serpapi.com",
	port: 443,
	};
	? {
	hostname: "localhost",
	port: 3000,
	protocol: "http",
	}
	: {
	hostname: "serpapi.com",
	port: 443,
	protocol: "https",
	};

[Ovi]

Let's include protocol here

Suggested change

	? {
	hostname: "localhost",
	port: 3000,
	}
	: {
	hostname: "serpapi.com",
	port: 443,
	};
	? {
	hostname: "localhost",
	port: 3000,
	protocol: "http",
	}
	: {
	hostname: "serpapi.com",
	port: 443,
	protocol: "https",
	};

[zyc9012]

Thanks @Ovi 👍

I removed the test against localhost instead. As this is an open-source repo, not all developers have SerpApi running in localhost.

[Ovi]

Thank you, @zyc9012! Works great! 👍
Just some minor tweaks.

[Ovi]

Thank you, @zyc9012!
Works great! 🚀