Skip to content

feat: fetch interface final #1255

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

Merged
merged 2 commits into from
May 19, 2022
Merged

feat: fetch interface final #1255

merged 2 commits into from
May 19, 2022

Conversation

janniks
Copy link
Collaborator

@janniks janniks commented May 12, 2022

cont'd changes

  • removes POC session middleware
  • refactorings (rename functions, extract functions, reduce nesting)
  • updates ts docstring comments
  • adds tests for middleware
  • remove default middleware (adding referrer header) in favor Require referrer header to be set #1246 (→ less points of failure / possible places to debug behavior)
  • moves fetch related functions to @stacks/common

example usage

Using createFetchFn we can now build an alternative to the default built-in fetchFn of stacks.js. We can pass middlewares to the createFetchFn that will hook in before a request or after a response. The createApiKeyMiddleware function generates a new middleware that will add a custom header to each request.

import { createApiKeyMiddleware, createFetchFn, StacksMainnet } from '@stacks/network';
import { broadcastResponse, getNonce, makeSTXTokenTransfer } from '@stacks/transactions';

const myApiMiddleware = createApiKeyMiddleware('example_e8e044a3_41d8b0fe_3dd3988ef302');
const myFetchFn = createFetchFn(apiMiddleware); // middlewares can be used to create a new fetch function
const myMainnet = new StacksMainnet({ fetchFn: myFetchFn }); // the fetchFn options can be passed to a StacksNetwork to override the default fetch function

const txOptions = {
  recipient: 'SP3FGQ8Z7JY9BWYZ5WM53E0M9NK7WHJF0691NZ159',
  amount: 12345n,
  senderKey: 'b244296d5907de9864c0b0d51f98a13c52890be0404e83f273144cd5b9960eed01',
  memo: 'some memo',
  anchorMode: AnchorMode.Any,
  network: myMainnet, // make sure to pass in the custom network object
};
const transaction = await makeSTXTokenTransfer(txOptions); // fee-estimation will use the custom fetchFn

const response = await broadcastResponse(transaction, myMainnet); // make sure to broadcast via the custom network object

// stacks.js functions, which take a StacksNetwork object will use the custom fetchFn
const nonce = await getNonce('SP3FGQ8Z7JY9BWYZ5WM53E0M9NK7WHJF0691NZ159', myMainnet);

Middleware can be used to hook into network calls before sending a request or after receiving a response.

import { createFetchFn, RequestContext, ResponseContext, StacksTestnet } from '@stacks/network';
import { broadcastResponse, getNonce, makeSTXTokenTransfer } from '@stacks/transactions';

const preMiddleware = (ctx: RequestContext) => {
  ctx.init.headers = new Headers();
  ctx.init.headers.set('x-foo', 'bar'); // override headers and set new `x-foo` header
};
const postMiddleware = (ctx: ResponseContext) => {
  console.log(await ctx.response.json()); // log response body as json
};

const fetchFn = createFetchFn({ pre: preMiddleware, post: preMiddleware }); // a middleware can contain `pre`, `post`, or both
const network = new StacksTestnet({ fetchFn });

// stacks.js functions, which take a StacksNetwork object will use the custom fetchFn
const nonce = await getNonce('SP3FGQ8Z7JY9BWYZ5WM53E0M9NK7WHJF0691NZ159', network);

@janniks janniks self-assigned this May 12, 2022
@vercel
Copy link

vercel bot commented May 12, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
stacks-js ✅ Ready (Inspect) Visit Preview May 19, 2022 at 1:16PM (UTC)

@janniks
Copy link
Collaborator Author

janniks commented May 13, 2022

I'm thinking about moving all fetch related functions to the @stacks/network package. I think they would fit better semantically, since the network package already includes networking (endpoints etc. in addition to the blockchain-network specific constants).
This would make later refactoring the network package easier. Also, I believe it makes more sense from a user point of view, compared to e.g. importing api-specific methods from @stacks/common.

What do you think @zone117x ?

@zone117x
Copy link
Member

I'm thinking about moving all fetch related functions to the @stacks/network package. I think they would fit better semantically, since the network package already includes networking (endpoints etc. in addition to the blockchain-network specific constants). This would make later refactoring the network package easier. Also, I believe it makes more sense from a user point of view, compared to e.g. importing api-specific methods from @stacks/common.

What do you think @zone117x ?

Yep makes sense to me!

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2022

Codecov Report

Merging #1255 (e76d6e9) into master (32c7d76) will increase coverage by 0.27%.
The diff coverage is 77.73%.

❗ Current head e76d6e9 differs from pull request most recent head 4af16a9. Consider uploading reports for the commit 4af16a9 to get more accurate results

@@            Coverage Diff             @@
##           master    #1255      +/-   ##
==========================================
+ Coverage   64.49%   64.76%   +0.27%     
==========================================
  Files         125      124       -1     
  Lines        8652     8702      +50     
  Branches     1859     1894      +35     
==========================================
+ Hits         5580     5636      +56     
+ Misses       2828     2815      -13     
- Partials      244      251       +7     
Impacted Files Coverage Δ
packages/common/src/index.ts 100.00% <ø> (ø)
packages/cli/src/network.ts 28.96% <25.00%> (-0.21%) ⬇️
packages/auth/src/userSession.ts 53.42% <50.00%> (-0.37%) ⬇️
packages/profile/src/profile.ts 68.59% <50.00%> (+0.26%) ⬆️
packages/network/src/network.ts 58.94% <58.94%> (ø)
packages/wallet-sdk/src/utils.ts 75.92% <62.50%> (-2.51%) ⬇️
packages/transactions/src/builders.ts 73.33% <65.38%> (+0.31%) ⬆️
...ages/wallet-sdk/src/models/legacy-wallet-config.ts 94.44% <66.66%> (-5.56%) ⬇️
packages/wallet-sdk/src/models/profile.ts 75.55% <80.00%> (-1.19%) ⬇️
packages/keychain/src/utils/index.ts 91.04% <90.90%> (+0.27%) ⬆️
... and 23 more

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 32c7d76...4af16a9. Read the comment docs.

@zone117x
Copy link
Member

@janniks could we get some usage examples for a few common use cases added to readme(s), like tx broadcast on testnet and mainnet, fee estimate on testnet and mainnet, and also added to this PR desc?

Copy link
Contributor

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

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

Good stuff Jannik/Matt 👍🏼

@janniks
Copy link
Collaborator Author

janniks commented May 18, 2022

@janniks could we get some usage examples for a few common use cases added to readme(s), like tx broadcast on testnet and mainnet, fee estimate on testnet and mainnet, and also added to this PR desc?

  • Added a section to the network readme (and copied to PR).

I'll start adding more example/docs context to PR descriptions — great convention!

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.

4 participants