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

fix: clientside router should use existing JsonRpcProvider obj #57

Merged
merged 3 commits into from
Jul 12, 2022

Conversation

kristiehuang
Copy link
Contributor

@kristiehuang kristiehuang commented Jul 12, 2022

Instead of reinstantiating provider obj from providerUrl, we want to pass in our existing JsonRpcProvider obj to routingApi. While rtk-query will cache args, it doesn't destroy the provider args - meaning our javascript garbage collector will collect it, which mean it may live in memory for longer than we want. (??)

Since we don't need time-travel debugging nor persistence, we can store non-serializable objects in state.

  • We turn off the serializable check for routing
  • Use a custom serializeQueryArgs fxn to generate a query cache key

@vercel
Copy link

vercel bot commented Jul 12, 2022

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

Name Status Preview Updated
widgets ✅ Ready (Inspect) Visit Preview Jul 12, 2022 at 8:39PM (UTC)

} else {
return value
}
})})`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

L24-38 are the default serialization fxn from rtk-query; I added extra case for our nonserializable provider arg

@kristiehuang kristiehuang changed the title feat: clientside router should use existing JsonRpcProvider obj fix: clientside router should use existing JsonRpcProvider obj Jul 12, 2022
@vercel vercel bot temporarily deployed to Preview July 12, 2022 20:37 Inactive
@vercel vercel bot temporarily deployed to Preview July 12, 2022 20:39 Inactive
@kristiehuang kristiehuang merged commit 8bb32f4 into feat/auto-router-params Jul 12, 2022
@kristiehuang kristiehuang deleted the router/use-provider-obj branch July 12, 2022 20:42
@@ -15,9 +18,29 @@ const DEFAULT_QUERY_PARAMS = {
protocols: protocols.map((p) => p.toLowerCase()).join(','),
}

const serializeRoutingCacheKey: SerializeQueryArgs<any> = ({ endpointName, queryArgs }) => {
// same as default serializeQueryArgs, but we add extra case if key is our non-serializable JsonRpcProvider
return `${endpointName}(${JSON.stringify(queryArgs, (key, value) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you still need to serialize this, now that you've disabled serialization checks.

kristiehuang added a commit that referenced this pull request Jul 21, 2022
* wip: feat: add autorouter param

* feat: add routerAPIBaseURL prop [wip]

* remove V3 quoters for useBestTrade

* feat: [wip] add rtk-query createAPI logic

* style(lint): lint action with ESLint

* feat: rename useBestTrade to useRouterTrade

* Add routingApi slice reducer to store

* use baseURL in API fetch

* use string type instead of URL type

* fix: use isFetching for trade state

* nit: remove comments

* Disable eslint for SOR import

* nit: add comments for router provider

* use activeWeb3React for router provider

* style(lint): lint action with ESLint

* nit

* nit: fix lint about React hooks

* nit: import comments

* remove jank logic

* fix: undefined chainId

* fix: use provider connection URLs for router provider

* nit: comments

* feat: combine useRouterTrade & useRoutingAPITrade

* nit: comments

* fix: rename params & address PR comments

* lint: type import SOR

* remove comments + address PR

* Remove 'useClientSideRouter' param

* fix: clientside router should use existing JsonRpcProvider obj (#57)

* [wip] feat: use provider obj in routingApi

* remove serialization check

* update comments

* remove comment

* update fxn docs

* nit: fix serializeQueryArgs type

* pr review

* nit: remove comment

* Add comments

* fix: check currency chainid only if token inputted

* feat: remove deprecated v2v3 quoters in useUSDCPrice (#58)

* style(lint): lint action with ESLint

* fix: Use clientside router for USDC prices

* fix: reolve circular dependency

* style(lint): lint action with ESLint

* fix: widget should not rerender on every new quote

* currentdata

* add cosmos routerUrl param

* fix: replace fetchBaseQuery

* linter

Co-authored-by: Lint Action <lint-action@uniswap.org>
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