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

beforeRequest doesn't merge options.json #1408

Closed
2 tasks done
nghiepdev opened this issue Aug 18, 2020 · 8 comments · Fixed by #1453
Closed
2 tasks done

beforeRequest doesn't merge options.json #1408

nghiepdev opened this issue Aug 18, 2020 · 8 comments · Fixed by #1453
Labels
enhancement This change will extend Got features ✭ help wanted ✭

Comments

@nghiepdev
Copy link

Hi,

I have custom options.json in beforeRequest hook, but it not working well.

Describe the bug

  • Node.js version: ^14
  • OS & version: macOS
  • Got: ^11.5.2

Actual behavior

import got from 'got';

const client = got.extend({
  prefixUrl: 'https://domain.com',
  hooks: {
    beforeRequest: [
      options => {
        if (options.method === 'post' || options.method === 'POST') {
          options.json = options.json ?? {};
          options.json.myValue = 'newValue';

          // Debug here: I see data correctly
          console.log(options.json);
        }
      },
    ],
  },
});

client.post('my-api", {
  json: {
    ok: "ok"
  }
});

In fact, myValue doesn't merge with options.json data.

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@szmarczak
Copy link
Collaborator

Because it's converted into options.body.

@szmarczak szmarczak added the question The issue is a question about Got label Aug 18, 2020
@nghiepdev
Copy link
Author

Is there ways to merge with options.json instance defaults?

@szmarczak
Copy link
Collaborator

Try this:

const client = got.extend({
  prefixUrl: 'https://domain.com',
  hooks: {
    beforeRequest: [
      options => {
        if (options.method === 'post' || options.method === 'POST') {
          options.json = options.json ?? {};
          options.json.myValue = 'newValue';
		  options.body = JSON.stringify(options.json);
        }
      },
    ],
  },
});

@szmarczak
Copy link
Collaborator

I think you should use the init hook instead. Let me know if that fixes the issue.

@nghiepdev
Copy link
Author

options.body = JSON.stringify(options.json); doesn't work.

I cant' use the init hook. Because the options.context in the init hook is undefined.
I need some data in context.

@szmarczak szmarczak added enhancement This change will extend Got features ✭ help wanted ✭ and removed question The issue is a question about Got labels Aug 18, 2020
@szmarczak
Copy link
Collaborator

I think we can pass a Proxy instance instead and normalize the options again if something changes.

@Giotino
Copy link
Collaborator

Giotino commented Aug 28, 2020

options.body = JSON.stringify(options.json); doesn't work.

Same problem here, also some explanations:
#1427 (comment)

@szmarczak
Copy link
Collaborator

options.body = JSON.stringify(options.json); doesn't work.

Ah... I just forgot that:

Because it's converted into options.body.

So you'd have to

const client = got.extend({
  prefixUrl: 'https://domain.com',
  hooks: {
    beforeRequest: [
      options => {
        if (options.method === 'post' || options.method === 'POST') {
          options.json = JSON.parse(options.body);
          options.json.myValue = 'newValue';
		  options.body = JSON.stringify(options.json);
        }
      },
    ],
  },
});

I cant' use the init hook. Because the options.context in the init hook is undefined.

In the upcoming release this will be possible. The init hook now accepts a second argument: self. So if context is defined in the instance, you can access context via self.context.


Yet another solution is to use the stringifyJson option.


Another way is to pass the body in the context and stringify it inside beforeRequest hooks. Note that you will have to update the content-length header as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This change will extend Got features ✭ help wanted ✭
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants