feat(icy-graphql-client): adds base skeleton of icy-graphql-client library [sc-39773] #6

AlejandroFabianCampos · 2022-08-30T22:12:35Z

Summary

Adds icy-graphql-client library to interface with developers.icy.tools

TODO

library documentation

shortcut-integration · 2022-08-30T22:12:37Z

This pull request has been linked to Shortcut Story #39773: Add one RPC function migration to SDK.

jkolyer

Can you include a command line shortcut for running tests plz?

I'm going to refactor the package.json scripts after I clean up the qn-ts scripts.

packages/libs/api/icy-graphql-client/testSetup/pollyTestSetup.ts

tools/scripts/publish.mjs

jkolyer

LGTM

My suggestion is we flag all known refactoring opportunities and do them in another iteration.

danieljvdm · 2022-09-09T19:21:58Z

packages/apps/examples/icy-graphql-client/.eslintrc.json

@@ -0,0 +1,18 @@
+{
+  "extends": ["../../../../.eslintrc.json"],


can we not just have these configs shared the root monorepo level?

probably yes, would need to test to double check, though I think it'd be fair to keep these in case any project needs to have any specifics (i.e. react hook linting or w.e.), they are automatically generated by nx to begin with so no effort is really being had for them to be created to begin with. WDYT?

huh I actually just learnt that specifically for eslint config, trying to override turn off the @typescript-eslint/ban-ts-comment rule that because of NX's base override of eslint, you need to override per project these rules or do more extensive eslint setup 🤦
nrwl/nx#5866

danieljvdm · 2022-09-15T14:14:35Z

packages/libs/api/icy-graphql-client/src/client/client.ts

+}
+
+const ICY_GRAPHQL_CLIENT_SUPPRESS_WARNINGS =
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment


probably just worth getting rid of this ESLINT rule if we're going to do this?

yeah I think that's fair, will add todo comment to keep track

applied on 084d13a!

danieljvdm · 2022-09-15T14:16:54Z

packages/libs/api/icy-graphql-client/src/client/icyGraphqlClient.ts

+    const result = await this.apolloClient.query(options);
+
+    if (result?.data && typeof result?.data === 'object') {
+      result.data = removeNodesAndEdges(result.data);


I think you can do this in an ApolloLink and not have to have this icyGraphQLClient. Basically as middleware and apply to all responses

Yup, I had the same thought going off of https://www.apollographql.com/docs/react/networking/advanced-http-networking/#modifying-response-data, but it seems like Apollo's in memory cache is run in between links (didn't get to dive fully on this, so might be slightly wrong on relation of errors) doesn't love that the actual structure and not just the scalar properties are modified apollographql/apollo-client#8677, so was getting errors left and right.
On a side learning, I also intended on directly extending Apollo's client and overriding the query method calling super and then modifying on the last mile, but it was causing typing issues as I learnt that you can't actually override the function signature (i.e. returning flattened data type) from the parent class; that's the reason for the total custom client that in itself delegates to the apollo client

hmm this makes sense, I briefly looked into it and yeah it seems like you might get some issues modifying the schema like this. So I think this is probably the correct layer to do it. In the past we've made our own useQuery hook so you don't have to declare a custom client, but obviously it's necessary for this.

Should we make the standard apollo client private in IcyGraphQLSDK? And I wouldn't be opposed to calling the icy client client and the apollo one baseClient or something if we want to ensure the "icy" one is used always instead of the base apollo one.

Should we make the standard apollo client private in IcyGraphQLSDK?

This is something I actually gave some thought, my feeling was that it could be helpful to expose the apollo client we build so that we can in the future instruct users to access that apollo client if they want specific data (and hence save them the hassle of setting up the http/error/auth links if they just want to get bootstrapped fast)
On second thought though, we could make it private as a property and set a custom named getter method if you think that'd be more clear? My logic on naming was a direct consequence of assuming that we'd be exposing both the "apollo" raw client and the "icy" higher level client

Note of color as well though, I was thinking that on our "Icy" higher level client we will probably not be 1:1 matching functionality the same as Apollo client, i.e. one improvement I was thinking along this line (longer term) is that we could return a "IcyResponse" instance as a superset of ApolloResponse with flattened data and magic pagination methods (i.e. response.nextPage())

my feeling was that it could be helpful to expose the apollo client

I meant make the standard client private, not the icy one.

When do you think users will need to use both clients? The icy one should just be a "superset" of the apollo one, no?

When do you think users will need to use both clients?

i.e. when a user wants to fetch some specific data or filter that is not fully covered by our abstraction SDK, they can access our raw apollo client and forward the sql query without having to do the auth/http/error link setup themselves

The icy one should just be a "superset" of the apollo one, no?

Technically no, because of not directly extending the apollo client as a consequence of method signatures being modified, but also it might be an opportunity to bring a more "graphql for dummies" sort of ORM approach

Note of color as well though, I was thinking that on our "Icy" higher level client we will probably not be 1:1 matching functionality the same as Apollo client, i.e. one improvement I was thinking along this line (longer term) is that we could return a "IcyResponse" instance as a superset of ApolloResponse with flattened data and magic pagination methods (i.e. response.nextPage())

danieljvdm · 2022-09-15T14:17:59Z

...libs/api/icy-graphql-client/src/queries/nft/getNFTsByWalletAddress/getNFTsByWalletAddress.ts

@@ -0,0 +1,38 @@
+import { gql } from '@apollo/client/core';
+import { PaginationArgs } from '../../../types';


Would prefer an absolute path here ... from '~/types' or whatever the path is

I had tried using basePath alias imports like 830fefe#diff-84aee5d7a1e271d85603f9d48b0100bd95edc3757d17a330eb5475dd8c99abe2L6 but it was causing issues when the code was directly reimported on other nx projects like on https://github.com/quiknode-labs/qn-oss/pull/6/files#diff-9d88951a6aa436ee6e66eb18c9215ab66c0f29e9cb8d6d5d674b668c2ddbc61aR1, it might be possible to fix with specific aliases or some top level ts config, but I was planning on punting this as an improvement for now, WDYT?

Punting is fine, but would like to be able to use absolute paths at some point here

danieljvdm · 2022-09-15T14:21:21Z

packages/libs/api/icy-graphql-client/src/utils/removeNodesAndEdges.ts

+/**
+ * @todo implement generic typing
+ */
+export const removeNodesAndEdges = (data: any): any | any[] => {


have fun typing this hehe

yeah haha, we'll have to get back to it

it's probably not really that necessary since i don't think you can ever use the output of this method in a context that retains types

…brary [sc-39773]

…ests be consistently replayable from recordings

…on other default package settings

…ractical dependency under apollo

…phql client

…ed response data flatenning for abstraction purposes

…d methods

…onse and fixes test specs

…eral usage information

…WalletAddress

…vigation across the monorepo

… and fixes tests for graphql client example

…le and build execution because of incorrect casing on import

…uickNode SDK

… renames CustomGraphqlClient -> CustomApolloClient

AlejandroFabianCampos self-assigned this Aug 30, 2022

AlejandroFabianCampos force-pushed the feat/sc-39773-add-graphql-client branch 2 times, most recently from be22ce6 to 895b7ee Compare September 6, 2022 23:54

AlejandroFabianCampos marked this pull request as ready for review September 9, 2022 18:34

AlejandroFabianCampos requested review from danieljvdm and jkolyer September 9, 2022 18:35

jkolyer reviewed Sep 11, 2022

View reviewed changes

packages/libs/api/icy-graphql-client/testSetup/pollyTestSetup.ts Outdated Show resolved Hide resolved

tools/scripts/publish.mjs Show resolved Hide resolved

jkolyer self-requested a review September 12, 2022 16:25

jkolyer approved these changes Sep 12, 2022

View reviewed changes

AlejandroFabianCampos force-pushed the feat/sc-39773-add-graphql-client branch 2 times, most recently from 821c291 to f7d276b Compare September 14, 2022 21:11

danieljvdm reviewed Sep 15, 2022

View reviewed changes

AlejandroFabianCampos force-pushed the feat/sc-39773-add-graphql-client branch from 6ddac45 to 084d13a Compare September 15, 2022 17:22

AlejandroFabianCampos added 13 commits September 19, 2022 10:38

feat(icy-graphql-client): adds base skeleton of icy-graphql-client li…

41d22f3

…brary [sc-39773]

chore(icy-graphql-client): adds polly to test setup to make test requ…

72d9a86

…ests be consistently replayable from recordings

chore(icy-graphql-client): adds publish logic and some minor changes …

e2ea791

…on other default package settings

chore(icy-graphql-client): adds babel and webpack full bundling config

26fd3b4

chore(icy-graphql-client): adds otp and minor fixes to remove react p…

6a8cfdb

…ractical dependency under apollo

chore(icy-graphql-client): adds example express app utilizing the gra…

7c0d458

…phql client

fix(icy-graphql-client): improvements on client api structure and add…

07fb533

…ed response data flatenning for abstraction purposes

fix(icy-graphql-client): standardizes camelcase naming on the exporte…

dada053

…d methods

fix(icy-graphql-client): removes __typename references on client resp…

f870eba

…onse and fixes test specs

chore(icy-graphql-client): improves readme with api reference and gen…

9bac8f7

…eral usage information

feat: splits up getNFTsByWallet onto getNFTsByWalletENS and getNFTsBy…

1d8a48d

…WalletAddress

chore: modifies root and package readme to improve readibility and na…

6d68ac7

…vigation across the monorepo

fix(icy-graphql-client): turns off eslint rule for ts-ignore comments…

4e80320

… and fixes tests for graphql client example

AlejandroFabianCampos force-pushed the feat/sc-39773-add-graphql-client branch from 084d13a to 4e80320 Compare September 19, 2022 13:38

AlejandroFabianCampos added 2 commits September 19, 2022 10:47

chore: fixes readmes' formatting

0bdf154

fix(icy-graphql-client): fixes lint execution on graphql client examp…

9ec55f7

…le and build execution because of incorrect casing on import

fix(sdk): removes all possible references of Icy Tools in favour of Q…

7fbaeb2

…uickNode SDK

AlejandroFabianCampos force-pushed the feat/sc-39773-add-graphql-client branch from 06e74b0 to 7fbaeb2 Compare September 20, 2022 15:18

fix(sdk): updates readme to reflect changes on the method interfaces,…

6c28a2c

… renames CustomGraphqlClient -> CustomApolloClient

AlejandroFabianCampos merged commit 67ce48e into main Sep 21, 2022

AlejandroFabianCampos deleted the feat/sc-39773-add-graphql-client branch September 21, 2022 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(icy-graphql-client): adds base skeleton of icy-graphql-client library [sc-39773] #6

feat(icy-graphql-client): adds base skeleton of icy-graphql-client library [sc-39773] #6

AlejandroFabianCampos commented Aug 30, 2022 •

edited

Loading

shortcut-integration bot commented Aug 30, 2022

jkolyer left a comment

jkolyer left a comment •

edited

Loading

danieljvdm Sep 9, 2022

AlejandroFabianCampos Sep 15, 2022 •

edited

Loading

AlejandroFabianCampos Sep 15, 2022

danieljvdm Sep 15, 2022

AlejandroFabianCampos Sep 15, 2022

AlejandroFabianCampos Sep 15, 2022

danieljvdm Sep 15, 2022

AlejandroFabianCampos Sep 15, 2022

danieljvdm Sep 19, 2022

AlejandroFabianCampos Sep 19, 2022

danieljvdm Sep 19, 2022 •

edited

Loading

AlejandroFabianCampos Sep 19, 2022

danieljvdm Sep 15, 2022

AlejandroFabianCampos Sep 15, 2022

danieljvdm Sep 19, 2022

danieljvdm Sep 15, 2022

danieljvdm Sep 15, 2022

AlejandroFabianCampos Sep 15, 2022

danieljvdm Sep 19, 2022

		@@ -0,0 +1,38 @@
		import { gql } from '@apollo/client/core';
		import { PaginationArgs } from '../../../types';

feat(icy-graphql-client): adds base skeleton of icy-graphql-client library [sc-39773] #6

feat(icy-graphql-client): adds base skeleton of icy-graphql-client library [sc-39773] #6

Conversation

AlejandroFabianCampos commented Aug 30, 2022 • edited Loading

Summary

TODO

shortcut-integration bot commented Aug 30, 2022

jkolyer left a comment

Choose a reason for hiding this comment

jkolyer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroFabianCampos Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieljvdm Sep 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlejandroFabianCampos commented Aug 30, 2022 •

edited

Loading

jkolyer left a comment •

edited

Loading

AlejandroFabianCampos Sep 15, 2022 •

edited

Loading

danieljvdm Sep 19, 2022 •

edited

Loading