Skip to content

chore: Migrate The Graph queries#5727

Merged
cbachmeier merged 6 commits into
apollo_migrationfrom
WEB-2584-handle-the-graph-queries
Dec 20, 2022
Merged

chore: Migrate The Graph queries#5727
cbachmeier merged 6 commits into
apollo_migrationfrom
WEB-2584-handle-the-graph-queries

Conversation

@cbachmeier
Copy link
Copy Markdown
Contributor

Migrate Pooled data queries from Relay to Apollo. Similar to how we previously had two different RelayEnvironments, I set up two Apollo clients. I also setup some middleware to dynamically update the uri for the requests. This was tested locally by creating pool positions and switching chains.

@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 19, 2022

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

Name Status Preview Updated
interface ✅ Ready (Inspect) Visit Preview Dec 20, 2022 at 2:12AM (UTC)

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Dec 19, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: aae33b6
Status: ✅  Deploy successful!
Preview URL: https://93192a10.interface-y3o.pages.dev
Branch Preview URL: https://web-2584-handle-the-graph-qu.interface-y3o.pages.dev

View logs

skip,
},
pollInterval: interval,
client: apolloClient,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

longer term, is there a way to share the client?

Copy link
Copy Markdown
Contributor Author

@cbachmeier cbachmeier Dec 19, 2022

Choose a reason for hiding this comment

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

I think there is a way by leveraging more middleware but it seems like the long term solution will be using our own endpoint which handles different chains. So I don't want to over invest in a more elegant solution if it would be thrown away https://uniswapteam.slack.com/archives/C02T729PMQE/p1671469080701009

uri:
chainId && CHAIN_SUBGRAPH_URL[chainId]
? CHAIN_SUBGRAPH_URL[chainId]
: CHAIN_SUBGRAPH_URL[SupportedChainId.MAINNET],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would this behave incorrectly if subgraph isn't defined for chain? should we throw?

alternatively you may be able to narrow the type of CHAIN_SUBGRAPH_URL with ts, as const

const CHAIN_SUBGRAPH_URL = {
  [SupportedChainId.MAINNET]: 'https://api.thegraph.com/subgraphs/name/uniswap/uniswap-v3',
  [SupportedChainId.RINKEBY]: 'https://api.thegraph.com/subgraphs/name/uniswap/uniswap-v3',
...
} as const

i think CHAIN_SUBGRAPH_URL[chainId] will never be undefined then

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This logic always falls back to Mainnet's chain. I also don't want to invest into changes to this logic if the long term plan is to rely on are in-house graphql data endpoint


useInterval(refreshData, interval, true)
return { error, isLoading, data }
return useMemo(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit. don't think you need to memo this one since data, loading and error should be stable from apollo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It has come about to make the standard practice for all our graphql queries to memoize the return data so while it might be stable I do like keeping with the standard personally

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought you were doing it to rename loading to isLoading

Comment thread .eslintignore
@@ -1,3 +1,4 @@
*.config.ts
*.d.ts
/src/graphql/data/__generated__/types-and-hooks.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should be given a better name than data, to contrast with thegraph.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

uniswap? Since it uses the Uniswap endpoint? Also I would prefer to refactor this after migrating to best keep changes localized to Relay -> Apollo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think uniswap makes sense. Also, fine to defer, I think avoiding scope creep is good here.

Comment thread package.json Outdated

useInterval(refreshData, interval, true)
return { error, isLoading, data }
return useMemo(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought you were doing it to rename loading to isLoading

@cbachmeier cbachmeier merged commit 66949c2 into apollo_migration Dec 20, 2022
@cbachmeier cbachmeier deleted the WEB-2584-handle-the-graph-queries branch December 20, 2022 16:20
cbachmeier added a commit that referenced this pull request Dec 20, 2022
* feat: initial apollo configutation (#5565)

* initial apollo configutation

* add new files

* check in types-and-hooks

* config unused export

* deduplicate

* ignore checked in schema for linting

* remove prettier ignore

* test unchecking types and hooks file

* undo

* rename codegen, respond to comments

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* Remove maybe value from codegen

* add babel gql codegen

* correct ts graphql-tag

* remove plugin from craco

* chore: migrate Assets Query to Apollo (#5665)

* chore: migrate Assets Query to Apollo

* delete comment

* move length check back to collectionAssets

* remove uneeded check

* respond to comments

* working switching and filters

* change sweep fetch policy

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* chore: migrate collection query to apollo (#5647)

* migrate collection query to apollo

* remove page level suspense

* undo removing page level suspense

* rename query and hook

* guard returns

* add return type prop

* cleanup nullables

* memoizing

* use gql from apollo

* use babel gql and move empty trait

* add fetch policy

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* chore: migrate NFT details query to apollo (#5648)

* chore: migrate NFT details query to apollo

* update todo

* update imports

* remove no longer used hook

* rename query

* use babel gql and nonnullable type

* working page

* add fetchpolicy

* respond to comments

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* chore: migrate NftBalanceQuery (#5653)

* chore: migrate NftBalanceQuery

* cleanup

* update pagination

* better undefined handling

* move brake listing for invalid asset higher

* better handle loading

* memoize and cleanup

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* remove named gql query consts

* set default fetchPolicy

* null suspense

* chore: Migrate The Graph queries (#5727)

* migrate TheGraph queries to Apollo

* add new files

* ignore thegraph generated types

* use standard fetchPolicy

* update apollo codegen commands

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* chore: migrate token queries to Apollo (#5682)

* migrate utils to types-and-hooks

* too many TokenTable re-renders

* working token queries

* fixed sparkline for native asset

* onChangeTimePeriod

* define inline

* use query instead of data in naming

* sparklineQuery instead of sparklineData

* rename to usePriceHistory

* multiline if else

* remove optional

* remove unneeded eslint ignore

* rename tokenQueryLoading

* rename OnChangeTimePeriod

* token address fallback

* just address

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* chore: deprecate Relay (#5747)

* chore: deprecate Relay

* remove graph:ql generate step

* add new files

* apollo to graphql centric naming

* add new files

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>

* remove no longer needed config exclusions

Co-authored-by: Charles Bachmeier <charlie@genie.xyz>
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.

3 participants