Skip to content

Set default value for an options object#1495

Merged
szmarczak merged 1 commit into
sindresorhus:masterfrom
mrmlnc:fix_unchangeable_options
Oct 9, 2020
Merged

Set default value for an options object#1495
szmarczak merged 1 commit into
sindresorhus:masterfrom
mrmlnc:fix_unchangeable_options

Conversation

@mrmlnc

@mrmlnc mrmlnc commented Oct 9, 2020

Copy link
Copy Markdown
Contributor

Hello,

I ran into a problem that after the client extension by the .extend method, the init hook gets an undefined instead of an object as options.

The following example illustrates the problem.

const got = require('got').default;

const client = got.extend({
	hooks: {
		init: [
			(options) => {
				console.dir({ options }, { colors: true });
			}
		]
	}
});

(async () => {
	await client('https://google.com'); // { options: undefined }
	await client('https://google.com', {}); // { options: {} }
	await client({ url: 'https://google.com' }); // { options: { url: <value> }}
})();

Looks like a bug, because I see that the init hook should get an Options object according to the signature in the types.

export type InitHook = (options: Options) => void;

So, this PR fixes the problem. If you accept this change, I can make a backport to the v11 branch.

Checklist

  • I have read the documentation.
  • I have included a pull request description of my changes.
  • I have included some tests.
  • If it's a new feature, I have included documentation updates in both the README and the types.

@mrmlnc
mrmlnc force-pushed the fix_unchangeable_options branch from 5578014 to 5b95af1 Compare October 9, 2020 18:10
@mrmlnc

mrmlnc commented Oct 9, 2020

Copy link
Copy Markdown
Contributor Author

Looks like CI fails due to last update of node.js. Works fine under 12.18.4 and breaks after 12.19.0.

  Uncaught exception in test/timeout.ts

  Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
  Please open an issue with this stack trace at https://github.com/nodejs/node/issues


  › Please open an issue with this stack trace at https://github.com/nodejs/node/issues
  › assert (internal/assert.js:14:11)
  › TCP.onStreamRead (internal/stream_base_commons.js:188:23)

@szmarczak
szmarczak merged commit 390b145 into sindresorhus:master Oct 9, 2020
@szmarczak

Copy link
Copy Markdown
Collaborator

@mrmlnc Can you please open another issue for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants