-
Notifications
You must be signed in to change notification settings - Fork 662
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(web-api): add request interceptor and HTTP adapter config to WebClient [ISSUE-2073] #2076
feat(web-api): add request interceptor and HTTP adapter config to WebClient [ISSUE-2073] #2076
Conversation
Thanks for the contribution! Before we can merge this, we need @mtjandra to sign the Salesforce Inc. Contributor License Agreement. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2076 +/- ##
==========================================
+ Coverage 91.75% 91.83% +0.07%
==========================================
Files 38 38
Lines 10093 10264 +171
Branches 631 646 +15
==========================================
+ Hits 9261 9426 +165
- Misses 820 826 +6
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the PR! I think this is heading in a great direction. I left many questions for my own education and to help inform my future reviews as well.
One higher-level question: what is the intention behind exposing both an adapter
option as well as requestInterceptor
? How/why would consumers use either/or? Would they use both in any situation? The request interceptor makes sense to expose, but based on the axios docs, it seems adapter
is mainly for use for testing (https://axios-http.com/docs/req_config)?
Also, I think we may need to add something to our docs about this new feature. Looking at the right menu bar for the web-api
docs here, perhaps a new section on request interceptors / modifiers would be in order? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! Left a few minor documentation suggestions; once applied or rejected, I will merge this in.
Thank you so much for your work here and patience with me during review, it is greatly appreciated 🙇
1c5eec0
to
2652028
Compare
…Data and remove data alias
2652028
to
5dabe0f
Compare
Thank you for helping me with this and being responsive! I've addressed your comments and have rebased with |
Summary
Resolves #2073
Features
Contribution Pre-Requisites