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

Update to axios 1.2.1 #217

Merged
merged 6 commits into from
Dec 20, 2022
Merged

Update to axios 1.2.1 #217

merged 6 commits into from
Dec 20, 2022

Conversation

ricellis
Copy link
Member

@ricellis ricellis commented Dec 9, 2022

Axios 0.x is not maintained any more and the 1.x stream contains some useful fixes (see also #209).
This PR is a cleaned up version of #213, but note that this PR requires #216 as a prerequisite (and since that is currently unmerged this PR also currently contains the commit from that PR). If there are axios-cookiejar-support changes then it may be possible to drop #216 and refactor this instead.

There are potentially breaking changes here because:

  • axios 1.x has a minimum of Node.js 14, so the minimum Node.js version here needs to increase (12.x is out of support)
  • axios 1.x defines types with TS 4.x syntax so the minimum typescript version must update (this could impact downstream SDKs that are not yet using TS 4.x)

Commit summary:

  • 994a25c - from refactor: cookie support via interceptors #216 PREREQ
  • e0b86d2 - BREAKING update to Node.js 14 minimum as required by axios 1.x
  • 8f16f08 - update axios dependency to 1.2.1
  • fd3ba09 - correct request-wrapper for axios 1.x
    • Use paramsSerializer legacy compatibility serialize function with existing stringify function.
    • Remove withCredentials from axios defaults.
    • Correct mocking in request-wrapper.test.js.
  • 41729f2 - BREAKING update typscript to 4.x (needed for axios 1.x)
  • 098dd62 - update Jest to 29.x (needed for axios 1.x)
  • e884861 - correct other test errors
    • build-request-file-object.test.js and get-content-type.test.js both make use of streams with non-existent paths to check errors. Possibly since upgrading jest these cause suites to fail with e.g.
    Error: ENOENT: no such file or directory, open '/fake/path/custom-name.env'
    Emitted 'error' event on ReadStream instance at:
        at emitErrorNT (node:internal/streams/destroy:157:8)
        at emitErrorCloseNT (node:internal/streams/destroy:122:3)
        at processTicksAndRejections (node:internal/process/task_queues:83:21) {
      errno: -2,
      code: 'ENOENT',
      syscall: 'open',
      path: '/fake/path/custom-name.env'
    
    • Since the errors are expected for these tests add a no-op handler to handle the error events without a warning
Checklist
  • npm test passes (tip: npm run lint-fix can correct most style issues)
  • tests are included
  • documentation is changed or added - updated README for Node.js 14 minimum

@ricellis ricellis changed the title Ricellis/axios 1.2.1 Update to axios 1.2.1 Dec 9, 2022
@ricellis ricellis mentioned this pull request Dec 9, 2022
3 tasks
@dpopp07
Copy link
Member

dpopp07 commented Dec 12, 2022

@ricellis This PR is looking great. I merged in the new cookie support so once you have this branch updated with the new cookie code, I can cut a new (potentially final) beta to test before merging this

BREAKING CHANGE: Minimum Node.js version is 14.

Upgrade to at least Node.js 14.

Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
BREAKING CHANGE: Minimum typescript version is 4.

Upgrade to typescript 4.x.

Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
build-request-file-object.test.js and get-content-type.test.js both make use of streams with non-existent paths to check errors. Possibly since upgrading jest these cause suites to fail with e.g.
```
Error: ENOENT: no such file or directory, open '/fake/path/custom-name.env'
Emitted 'error' event on ReadStream instance at:
    at emitErrorNT (node:internal/streams/destroy:157:8)
    at emitErrorCloseNT (node:internal/streams/destroy:122:3)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/fake/path/custom-name.env'
```

Since the errors are expected for these tests add a no-op handler to handle the error events.

Signed-off-by: Rich Ellis <ricellis@users.noreply.github.com>
@ricellis ricellis marked this pull request as ready for review December 13, 2022 09:22
@ricellis
Copy link
Member Author

Thanks @dpopp07 - I've updated this ready for another round of beta testing that definitely seems like a good plan!

@dpopp07
Copy link
Member

dpopp07 commented Dec 13, 2022

I've updated this ready for another round of beta testing that definitely seems like a good plan!

Great - the new beta has been published! Let me know how testing goes. If all is well, I'll merge tomorrow

@ricellis
Copy link
Member Author

@dpopp07 our tests are still looking green with beta-5 though given the nature of the change it would be more than a little reassuring to see feedback from any other SDKs too 😅

@dpopp07
Copy link
Member

dpopp07 commented Dec 15, 2022

our tests are still looking green

Great!

given the nature of the change it would be more than a little reassuring to see feedback from any other SDKs too

Great point. We spent some time yesterday doing additional testing with the Platform Service SDKs and everything looks green on that end too. I think we're looking good to ship

Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for all of your work on this and for crafting your commit messages so well 👍

@dpopp07 dpopp07 merged commit 6979711 into main Dec 20, 2022
@dpopp07 dpopp07 deleted the ricellis/axios-1.2.1 branch December 20, 2022 16:14
@ibm-devx-sdk
Copy link

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants