Request params are not getting merged with instance params

[zackseuberling]

Describe the bug

Specific request params do not get merged with the instance default params.

To Reproduce

const instance = axios.create({
    baseURL: "http://www.example.com",
    params: {
      q: "question",
    }
  });

instance.get("/page", 
  { 
    params: { 
      page: 2 
    }
  }
)

What happens is that the request param object overrides the instance default param object. The instance no longer has a query param of q.

Expected behavior

According to the docs, all configuration options are merged. https://github.com/axios/axios#instance-methods

I expected request to contain both q and page params. This is also how it was working in version 0.18.0, which is how I noticed.

Environment:

• Axios Version: 0.19.0
• OS: 10.14.5 (18F132)
• Browser: Node Express, and Chrome
• Browser: Node 10.15.0, Chrome Version 74.0.3729.169 (Official Build) (64-bit)

[philipbjorge]

Here's a running example -- https://runkit.com/philipbjorge/axios-regression

[kylane]

Having a similar issue with our session based auth tokens not being merged either.

[zackseuberling]

I believe the fix is as simple as moving params to the property to the deepMergeProperties section. Diff here: 8d0b92b...zackseuberling:zs-fix-merge-params

[jasonsaayman]

@zackseuberling could you put in a pull request for this?

[flynnpark]

I have a same issue.

[zackseuberling]

@jasonsaayman tests are now passing (for what I assume the correct behavior is) in #2196

[JennerChen]

hope release soon , the issue made me pretty annoying

[CloudPower97]

Having the same issue. When is it going to be fixed?

[serranoarevalo]

Same here @wphestiraid 😬

[Vpr99]

Downgrading to 0.18.1 fixed it for me as a workaround! That has the security vulnerability fixed.

[flynnpark]

@serranoarevalo I think we can use 0.18.1 😄

[alex-at-cascade]

This seems to also happen with method, if it's only specified in the create call and not the request. Took me all day to figure that out, definitely a breaking change. (Should this be a separate issue, or is it part of the same general problem?)

[techouse]

Same issue here

[oussemamzoughi]

any news about this one ? when it will be fixed ?

[techouse]

any news about this one ? when it will be fixed ?

Just use v0.18.1 for the time being. ✈️

[adamreisnz]

When can we expect this elusive 0.20 release? Is there a beta version we can use in the mean time?

[jasonsaayman]

@adamreisnz I am working on the 0.20 release but there is still about 60 odd pull requests (brought this down from 120 odd), with some being very old and varied amount of work in checking and making sure these pull request either are still relevant, don't present a breaking change, have tests, don't include compiled files etc etc it is a labour. I endeavour to try get it into a pre-release by July at best. We would love help testing this release as we really want to get it right before we work on a v1.

[OctupleSakura]

It is an annoying problem, hope it can be fixed, is maintained for a long time.

[markhalliwell]

want to get it right before we work on a v1

There's plenty of room in 0.x to make additional releases before 1.0.0; you don't have to cram everything into the next release. In fact, one could argue that would just make potential new bugs just harder to track down.

[jasonsaayman]

@MarkCarver Sure we could however Axios is seen as pre-release currently as without a v1 it looks to most people that Axios has not released a public API see SemVer PublicAPI. With v0.20.0 we would like to bring stability and fix most regressions from the 0.19.x branch as some problems presented in this branch as well as work on errors that are easy wins.

We also do need to clear the backlog of pull requests at the same time. It is very difficult to deal with 120 odd pull request where some of them were from as far back as 2017. I promise we will get to this issue, I am going nowhere and will be supporting and working on Axios as much as I can.

Thanks

[adamreisnz]

I think there's just a lot of frustration building up, because we can't use certain plugins (e.g. axios cache adapter) unless we downgrade and lock in Axios at v0.18, or roll our own (unmaintained) version and apply the fix there.

This has been the case for quite a few months now, and I think everyone is just keen to see at least some fixes released.

Is it not possible to port some of the currently applied fixes and release a 0.19.x so that at least we can use the latest codebase? If you intend to go through another 120 pull requests, I am afraid we'll be waiting a long time until we see a 0.20 or v1.

I would personally opt for a more incremental approach, if possible.

[jasonsaayman]

@adamreisnz sure I would opt for increments too but there was alot broken from 0.18.x to 0.19.x and the idea was to try make a 0.20.0 release stable with some of the most pressing stuff in that release. We only have 60 pull requests left, I have already worked through the rest and some of those 60 are either tagged as wip or v1.

I know it has been a long time but I would like to get it right this time and not just release and break more things cause at this point that is largely possible. Also the build will need wide spread testing before it goes public. I get the frustration but if we don't get it stable and fix a ton of regressions we could release any number of increments, and still sit with over 200~ issues and 60~ pull requests that just sit there getting stale, as well as a still very broken Axios, maybe even more broken.

In addition I don't control releases at the moment and when I started helping I promised to get together a build that would fix regressions and the most pressing issues as well as not breaking anything else that worked previously. I take this pretty serious and would like to do that.

I can however try and get a 0.18.x release with any security issues etc fixed if that is something that would help?

[techouse]

I can however try and get a 0.18.x release with any security issues etc fixed if that is something that would help?

That would be awesome.

P.S.: We all appreciate the hard work you're doing by maintaining a package that is intricate to so many web projects. 😊

[adamreisnz]

I can however try and get a 0.18.x release with any security issues etc fixed if that is something that would help?

Yes I concur, that would definitely be appreciated.

If you need any help testing some of the new stuff let me know I'd be open to doing that.

[jasonsaayman]

@adamreisnz could you test this branch: 0.20.0-beta.1

[adamreisnz]

I can yes, will try to switch to that branch this weekend for one of our projects.

[adamreisnz]

@jasonsaayman so far so good, 0.20.0 seems to be working well 👍

[heyqbnk]

It seems, issue can be closed now

[jasonsaayman]

Closing after receiving feedback that this is now working as it should under the new release.

[kallezz]

Still having this issue in 0.24.0.

const api = axios.create({ baseURL: url, params: { api_key, }, });

api(/xxxxx, { method: "GET", params: config.query, });

config.query contains page param. It does not get merged with the instance params.

[simoneb]

@linchen5913 can you provide a repro please?

[linchen5913]

@linchen5913 can you provide a repro please?

@simoneb Oops, I happened to replace the whole params with
axios.interceptors.request.use(config=>{config.params={query:"foo"}})
I should be using the following code instead:
axios.interceptors.request.use(config=>{config.params={query:"foo", ...config.params}})
which solved the problem

Thank you so much for your time and response, I made a mistake here.

[johncmunson]

Closing after receiving feedback that this is now working as it should under the new release.

Was the intent to merge params with the default instance params? Because I can definitively say that this is not happening. The default params are being completely overridden. I'm using ^1.4.0.

Using interceptors is fine and all. That works. It's just pretty unintuitive that axios instances don't merge the runtime config with the instance config.