Fix moxios by not forcing adapters to resolve baseURL

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

Fixes #2650

Adapters should not have to care about baseURL; everything should be fully resolved by the point it gets to the adapter. The original PR (#2391) broke moxios tests that used baseURL in the production code, but stubbed requests using full URLs.

This PR should maintain the behavior the first PR set out to fix (namely, that the config URL is not modified) while allowing adapters to have the same behavior as before the first PR. In general, I recommend not modifying the config at all in dispatchRequest, but this fix should suffice for the issue above

[chinesedfan]

The changes of config is one of the most annoying things in axios. It happens in several files, and no clear documents record them. I think we should reach a consensus first, and write lots of tests to cover as more cases as possible.

In adapters, I agree that they should not care about things like combining baseURL with url. But will some users want to get the original config.url by response.config.url?

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

But will some users want to get the original config.url

I say wait until we see a use case. If one comes up, we could always add a rawUrl field, but I don't think we need to be proactive with this until we see a need for it

[floriangosse]

Any decision made for that? Can I help somehow?

[chinesedfan]

This PR should maintain the behavior the first PR set out to fix

@brandon-leapyear Can you add a test case for #1628? I don't think you kept that. Because error.config received in response interceptor is the config in adapters, which is modified.

@floriangosse You can help test or give any opinions you have. We are glad to hear more suggestions.

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

The PR fixing #1628 added a test that I didn't change

https://github.com/multicolaure/axios/blob/4f546cc415f72b5c5efc664939db82d6daf0d3bf/test/specs/requests.spec.js#L248

[chinesedfan]

@brandon-leapyear That's why CI is failed now.

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

@chinesedfan ah got it. How does that fix look?

[chinesedfan]

I say wait until we see a use case. If one comes up, we could always add a rawUrl field, but I don't think we need to be proactive with this until we see a need for it

@brandon-leapyear I realized that #1628 was the case. Now the test passed but was not satisfied with #1628. Adding rawUrl is one of possible solutions. And hope you or someone can find out an ideal solution without extra field.

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

@chinesedfan the more I think about this, the more I think avoiding config modification is the correct solution. Then, the error that interceptors receive could have both the rawConfig that was first passed into axios, and the config that was actually resolved.

It solves #1628 in the sense that the original goal was to rerun the request, using the same config initially passed into the request. This goes beyond the baseURL/url issue; even after "solving" #1628 with the initial solution, config modifications would happen twice, e.g. config.data = transformData(...). It just so happens that the baseURL/url issue is the most visible way that this is broken

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

Blocked by #2798

[yujihu]

I think it should be consistent because many adapters rely on config.url. If the config.url changes, it's destructive to them.

[Oripi]

any updates on this?

[mzabriskie]

A recent code change to moxios now takes into account config.baseURL. This will ought to be considered if this change is made. See axios/moxios#25

[chinesedfan]

Holy shit! It's @mzabriskie. Will you come back to lead axios or give me 30mins to discuss its future? You can find me on GitHub/Gitter and slacks.

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

ah thanks @mzabriskie. This PR can be closed, in that case, although I highly recommend #2798 anyway, to avoid modifying configs as it gets passed around

[jasonsaayman]

Closing