some defaults are not being respected in normalizeArguments

[ghermeto]

Describe the bug

• Node.js version: 10, 12, 14
• OS & version: MacOS

There is a problem with the function normalizeArguments related to prefixUrl where defaults are not being applied.

Actual behavior

The following test fails with TypeError: No URL protocol specified:

test('extending prefixUrl', withServer, async (t, server) => {
	server.get('/', (req, res) => {
		res.json(req.headers);
	});

	const instance1 = got.extend({
		prefixUrl: server.url
	});

	const instance2 = got.extend({
		headers: { 'x-test': 'test' },
		responseType: 'json'
	});

	const merged = instance1.extend(instance2);

	const {body} = await merged.get('');
	t.is((body as unknown as Record<string, unknown>)['x-test'], 'test');
});

However, if merged is defined as:

const merged = instance2.extend(instance1);

the test will pass. It seems like some defaults are not being properly applied because when some properties are not defined, it is set them to an empty string. So, when the next instance performs a shallow merge with the object spread, it will prefer the empty string.

Expected behavior

I should be able to define merge both as instance1.extend(instance2) and instance2.extend(instance1)

Quick Patch

Adding the following code to L1610 in /source/core/index.ts will solve the issue, but other properties might also be affected by the same issue.

// !options.prefixUrl checks if value is falsy (includes empty string and 0)
if (defaults?.prefixUrl && !options.prefixUrl) {
    options.prefixUrl = defaults.prefixUrl;
}

I will check the other properties and I can submit a PR later this week.

Checklist

• I have read the documentation.
• I have tried my code with the latest version of Node.js and Got.

[Giotino]

I will check the other properties and I can submit a PR later this week.

prefixUrl is a special case because its default is an empty string and this is enforced during normalization.

In your example instance1.prefixUrl is server.url and instance2.prefixUrl is '', but '' is not treated as "to overwrite" from the normalizeArguments function.
This PR will fix it #1448

[szmarczak]

@ghermeto If you expect to ignore Got defaults when merging, let's continue the discussion at #1450

[ghermeto]

that is perfect, thank you.