Skip to content

Conversation

@benflap
Copy link
Contributor

@benflap benflap commented Nov 23, 2023

Fixes: #143

Axios is not a drop-in replacement for Popsicle. Here are a few of the API differences:

  • Request config:
    • Axios uses data instead of body.
    • Axios.get() will override the the method option.
    • There is no transport option.
  • Response schema:
    • Headers are turned into an object with lower cased names.
    • Response data is atomically parsed and can be accessed with res.data.
    • The response can't be stringified using JSON.stringify() because of res.request.

Breaking Change

Given the transport issue, this PR does introduce a breaking change. Users that receive a PDF will have to change their code from this:

oauthClient.makeApiCall({
  url: `${url}v3/company/${companyID}/invoice/${invoiceNumber}/pdf?minorversion=59`,
  headers:{'Content-Type': 'application/pdf','Accept':'application/pdf'},
  transport: popsicle.createTransport({type: 'buffer'})
})

To this:

oauthClient.makeApiCall({
  url: `${url}v3/company/${companyID}/invoice/${invoiceNumber}/pdf?minorversion=59`,
  headers:{'Content-Type': 'application/pdf','Accept':'application/pdf'},
  responseType: 'arraybuffer'
})

In my opinion, switching to Axios is still worth it.

Testing

The only things I use this package for are creating redirects and creating tokens, so those are the only things I've tested. Other users should test out the rest of the package before this PR is merged.

@rajeshgupta723
Copy link
Collaborator

rajeshgupta723 commented Feb 19, 2024

Thanks for raising this PR for migrating to Axios from Popsicle. Few comments,

  • I tested the sample client with your branch code and it seems to be working fine. However, as it introduces some breaking changes, the overall impact needs to be assessed and tested.
  • In terms of popularity (# of downloads), agree that Axios popularity is pretty high (as per https://npmtrends.com/axios-vs-popsicle-vs-request ).
  • Axios release 1.x.x with ~500 outstanding issues (as per https://www.npmjs.com/package/axios) raises some questions on the stability of Axios ver 1.x.
  • Overall, we will continue to watch on this migration/merge as it requires further analysis.

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