Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

got.extend() doesn't merge hooks #606

Closed
dima-takoy-zz opened this issue Sep 8, 2018 · 6 comments
Closed

got.extend() doesn't merge hooks #606

dima-takoy-zz opened this issue Sep 8, 2018 · 6 comments
Labels
bug Something does not work as it should

Comments

@dima-takoy-zz
Copy link

I am try to get options dict for modifing it later. But looks like hook never been called.

'use strict'
const got = require('got')

const client = got
  .extend({
    baseUrl: 'https://jsonplaceholder.typicode.com/todos/1',
    json: true,
    hooks: {
      beforeRequest: [(options) => console.log(111111111111)],
    },
  })
  .get('/')
  .then(console.log)
  .catch(console.error)

got version: 9.2.1
node version: v10.10.0

@dima-takoy-zz
Copy link
Author

dima-takoy-zz commented Sep 8, 2018

hm, if i pass hook to .get('/') function then it works.

const got = require('got')

const noop = () => {}

const hooks = {
  beforeRequest: [console.log],
}

const client = got.create({
  options: {
    baseUrl: 'https://google.com',
    hooks,
  },
})

// not works
client
  .get('/')
  .then(noop)
  .catch(noop)

// works
client
  .get('/', { hooks })
  .then(noop)
  .catch(noop)

@szmarczak
Copy link
Collaborator

Can you elaborate where's the problem?

@dima-takoy-zz
Copy link
Author

dima-takoy-zz commented Sep 8, 2018

How i can define global hooks?

when i replace handler i got "compiled" options, where query already attached to url.

So my solution is wrong. It just replaces hooks(as described here) https://github.com/sindresorhus/got#gotmergeoptionsparentoptions-newoptions

@szmarczak
Copy link
Collaborator

What do you mean by "global hooks"?

@dima-takoy-zz
Copy link
Author

What do you mean by "global hooks"?

Hooks that works for all children of instance. With extend we can add hooks. We can also manage hooks inside hooks. Now, when i use extend hooks are just replaces.

Can you elaborate where's the problem?

'use strict'

const got = require('got')

const noop = () => {}

const hooks = {
  beforeRequest: [console.log],
}

const client = got.create({
  options: {
    baseUrl: 'https://google.com',
    hooks,
  },
})

// hooks here, OK!
console.log(client.defaults.options.hooks)

// but it not works, hook not called
client
  .post('/')
  .then(noop)
  .catch(noop)

// when i pass directly hooks will be called
client
  .post('/', { hooks })
  .then(noop)
  .catch(noop)

@szmarczak szmarczak changed the title hooks never been called got.extend() doesn't merge hooks Sep 8, 2018
@szmarczak szmarczak added the bug Something does not work as it should label Sep 8, 2018
@szmarczak
Copy link
Collaborator

szmarczak commented Sep 8, 2018

Confirmed bug:

test('hooks are merged on got.extend()', t => {
	const hooksA = [() => {}];
	const hooksB = [() => {}];

	const instanceA = got.create({options: {hooks: {beforeRequest: hooksA}}});

	const extended = instanceA.extend({hooks: {beforeRequest: hooksB}});
	t.deepEqual(extended.defaults.options.hooks.beforeRequest, hooksA.concat(hooksB));
});

EDIT: fixed the test

Difference:

    [
      Function {},
  +   Function {},
    ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should
Projects
None yet
Development

No branches or pull requests

2 participants