Skip to content

feat: Ability to set custom fetch function as an option on HTTP transport#301

Closed
bradjones1 wants to merge 4 commits intoopen-rpc:masterfrom
bradjones1:custom-fetch
Closed

feat: Ability to set custom fetch function as an option on HTTP transport#301
bradjones1 wants to merge 4 commits intoopen-rpc:masterfrom
bradjones1:custom-fetch

Conversation

@bradjones1
Copy link

I am implementing a native app with React Native and use OAuth for authentication. To facilitate token refresh on demand, I am using https://github.com/badgateway/fetch-mw-oauth2 which provides a fetch middleware to retry failed requests after refreshing access tokens.

This pattern works well when clients allow setting a custom fetch function, e.g. in the case of Orbit.js (a json:api client.)

I am not really much of a TypeScript developer but I have worked this out locally and it "seems to work." Hoping this could be an easy/backwards and forwards compatible change that would make this library even better. Thanks!

interface HTTPTransportOptions {
credentials?: CredentialsOption
headers?: Record<string, string>
fetch?: any
Copy link
Author

Choose a reason for hiding this comment

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

I'm not really sure what to put here (not a typescript guy...) but the linter didn't like Function

@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #301 (dd8d207) into master (bb0ea71) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #301   +/-   ##
=======================================
  Coverage   99.26%   99.27%           
=======================================
  Files          11       11           
  Lines         410      411    +1     
  Branches       62       63    +1     
=======================================
+ Hits          407      408    +1     
  Misses          3        3           
Impacted Files Coverage Δ
src/transports/HTTPTransport.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1c9b04...dd8d207. Read the comment docs.

@bradjones1 bradjones1 changed the title Ability to set custom fetch function as an option on HTTP transport feat: Ability to set custom fetch function as an option on HTTP transport Dec 13, 2021
@BelfordZ BelfordZ requested review from shanejonas and zcstarr July 5, 2022 20:44
@zcstarr
Copy link
Member

zcstarr commented Jul 6, 2022

Hey sorry for the delayed response on this, but I think what you want to do here is just make a custom transport. If it's not problematic for you and reactnative you could even just extend HttpTransport and then overwrite the connect method.

 class CustomHttpTransport extends HTTPTransport {

  connect(){
   ....
  }
}

Then usage within the client context is

const transport = new CustomHTTPTransport("http://localhost:8545");
const client = new Client(new RequestManager([transport]));
const result = await client.request({method: "addition", params: [2, 2]});

If you need access to it from another part of the stack such as the generator, for now because CustomHttpTransport extends HTTPTransport, you should be able to pass the custom transport in from the generated client data.

It's possible that we've typed it too strictly from the generator side, if so could you put up an issue https://github.com/open-rpc/generator

Thanks for the PR, but I think the proper thing here is a custom transport

@bradjones1
Copy link
Author

OK, thanks for the feedback.

@bradjones1 bradjones1 closed this Jul 6, 2022
@bradjones1
Copy link
Author

This got done in basically a similar way at #330

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