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

feat: upgrade axios #18

Merged
merged 3 commits into from
Jul 8, 2024
Merged

feat: upgrade axios #18

merged 3 commits into from
Jul 8, 2024

Conversation

osslgtm
Copy link

@osslgtm osslgtm commented Jul 1, 2024

upgrade axios

@osslgtm osslgtm changed the title upgrade axios chore: upgrade axios Jul 1, 2024
@osslgtm osslgtm force-pushed the feat/upgrade-axios branch from 69f9f9e to d1ed7cf Compare July 3, 2024 08:57
@osslgtm osslgtm changed the title chore: upgrade axios feat: upgrade axios Jul 3, 2024
@osslgtm
Copy link
Author

osslgtm commented Jul 3, 2024

(updated commit message)

@rongquan1
Copy link

@osslgtm

Could you try to test these changes by importing them into a new application and verify that the functions calling axios are working correctly? In particular, the queryDns function.

For more context, the creator website is not working with these changes.

@osslgtm
Copy link
Author

osslgtm commented Jul 5, 2024

@rongquan1
https://github.com/osslgtm/test-dnsprove/

(added in protocol check in case https:// or http:// is used on dnsquery)

is there something specific that broke ?

@rongquan1
Copy link

@osslgtm

Could you please take a look at the document-creator-website repo and try to import these changes?

We are encountering the following error: TypeError: (0 , _axios.default) is not a function.

@osslgtm
Copy link
Author

osslgtm commented Jul 5, 2024

is it on the editor side or during the build process ?

seems to work when importing it locally

"dependencies": {
  "@tradetrust-tt/dnsprove": "file:../dnsprove",
  "axios": "^1.7.2",
}

build and lint works as well

@rongquan1
Copy link

@osslgtm

This is happening at runtime when we try to click a button on the UI which is supposed to call the function that triggers the axios.

@osslgtm
Copy link
Author

osslgtm commented Jul 8, 2024

Fixed, can try again

Copy link

@rongquan1 rongquan1 left a comment

Choose a reason for hiding this comment

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

Thanks! Can you update the tt-verify repo after this package is published?

@rongquan1 rongquan1 merged commit 2e79cb8 into master Jul 8, 2024
5 checks passed
@osslgtm
Copy link
Author

osslgtm commented Jul 8, 2024

sure!

@osslgtm
Copy link
Author

osslgtm commented Jul 8, 2024

the release seems to be failing, seems to be an npm token issue

Copy link

github-actions bot commented Jul 9, 2024

🎉 This PR is included in version 2.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@rongquan1
Copy link

@osslgtm package just released

@rongquan1
Copy link

@osslgtm

Using relative path import axios from "../node_modules/axios/index"; might cause some issues if it is not there. Will be reverting this changes.

@osslgtm
Copy link
Author

osslgtm commented Jul 17, 2024

@rongquan1

Do you have details on the issue happening ?

Would you prefer if we patch the issue or use an alternate package ?

Any info would be appreciated

@rongquan1
Copy link

@osslgtm

We are open to using an alternative package to replace axios if it cannot be upgraded.

However, we must ensure that the replacement maintains all existing functionality.

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.

2 participants