yTakkar / fighting-boredom

MIT License
0 stars 0 forks source link

Core api sketch #1

Open phenax opened 5 years ago

phenax commented 5 years ago

@yTakkar @rakshans1, let's re-iterate on the core api sketch here. If you guys have any questions or have any suggestions, comment them here.

interface CacheAdapterTypeClass {
  getItem(k: string): Promise<any>
  setItem(k: string, v: any): Promise<any>
}

type Options = {
  endpoint: string
  cacheAdapter?: CacheAdapterTypeClass
  fetch?: typeof (window.fetch) // fetch option is to allow using the client in node platform as well
};

// endpoint is required. The rest are optional (values are default parameters).
const options: Options = {
  endpoint: 'https://someapi.com/graphql',
  cacheAdapter: MemoryCache(),
  fetch: window.fetch,
};

const gql = GraphQL(options);

const query = gql.query`
  query RepoInfo($user: String!, $repo: String!) {
    user(login: $user) {
      repository(name: $repo) {
        name
        description
      }
    }
  }
`;

const response = await query.run({
  user: "username",
  repo: "some-repo"
});
phenax commented 5 years ago

First problem with the sketch is gql.query api. GraphQL has queries and mutations so do we keep gql.query and gql.mutation as two different functions or do we have a unified function for it? I'm not sure the logic for the two is that different. If it is then we will need to use two different functions.

yTakkar commented 5 years ago

@phenax I agree with you. If query and mutation implementations are kind of identical then we should have the same function.

I was reading urql's code and found out that they have all core implementations in one place client.js and every feature such as query and mutation require them making the code implementation flow from 1 place only.

So we can have core methods such preparing request, executing request, etc.. in that factory function/class.

Here they have code methods and is used in

I'll suggest you guys go over their architecture.

@phenax @rakshans1 what your thoughts on this?

yTakkar commented 5 years ago

Are we going to support subscriptions also along with Query and Mutations?

phenax commented 5 years ago

@yTakkar, We can work on subscriptions but as a second entry point because subscriptions doesn't seem like that common of a usage feature when it comes to graphql.

An article for reference -> https://hackernoon.com/from-zero-to-graphql-subscriptions-416b9e0284f3

yTakkar commented 5 years ago

Yeah agree with you. Query and Mutations cover almost all usecases.

yTakkar commented 5 years ago

Not so sure about cacheAdapter workings. Can you please explain a bit. @phenax

phenax commented 5 years ago

@yTakkar, It'll be a simple generic interface for interacting with a cache. A few we can try are MemoryCache, LocalStorageCache, IDBCache, etc.

I was thinking something that looks like browser ka localStorage api with promises. Their api would look something like...

const cache = MemoryCache();
const world = await cache.getItem('hello');
await cache.setItem('hello', 'world2');

So our client wont care what cache it is as long as it has that api, it will just work. So in the server if someone wants to use this library, they can integrate it with redis too.

phenax commented 5 years ago

Also, we should have a way to invalidate this. Something like a versioning system that the constructor takes. It will just prefix that to the cache key.

const cache = MemoryCache('v1');
yTakkar commented 5 years ago

Just a thought.

// Just a simple Queries object
const q = {
    reposCount: () => `
        query RepoInfo($user: String!) {
          user(login: $user) {
            id,
            avatarUrl,
            repositories {
              totalCount
            }
          }
        }
    `,
    repoInfo: () => `
        query RepoInfo($user: String!, $repo: String!) {
            user(login: $user) {
              repository(name: $repo) {
                name
                description
              }
            }
          }
    `
}
(async () => {
    const reposCountQuery = gql.query(q.reposCount())
    const reposCountResponse = await reposCountQuery.run({ user: "yTakkar" })

    const repoInfoQuery = gql.query(q.repoInfo())
    const repoInfoResponse = await repoInfoQuery.run({ 
        user: "yTakkar", 
        repo: "something" 
    }) 
})()

With below implementation, I no longer had to create reposCountQuery and repoInfoQuery.

(async () => {
    const reposCountResult = await qgl.query({ 
        q: q.reposCount(),  
        variables: { user: "yTakkar" }
    })

    const repoInfoResponse = await qgl.query({ 
        q: q.repoInfo(),  
        variables: { user: "yTakkar", repo: "something" }
    })
})()

Or is there are any other reason to have run as a key on the returned object from query.

phenax commented 5 years ago

The reason I sketched it that way was to use tagged template string literals to our advantage.

Like for example in case of fragments we can do stuff like

const repoStarcountFragment = gql.fragment`
  fragment RepoParts on Repository {
    stargazers(first: 0) {
      totalCount
    }
  }
`;

const stargazerCountQuery = gql.query`
  query StarsCount($user: String!, $repo: String!) {
    user(login: $user) {
       repository(name: $repo) {
      ${repoStarcountFragment}
       }
    }
  }
`;

stargazerCountQuery.run(vars);

The tagging function will normalize the fragment into the query itself so the final query being sent in the request will look something like

  fragment RepoParts on Repository {
    stargazers(first: 0) {
      totalCount
    }
  }
  query StarsCount($user: String!, $repo: String!) {
    user(login: $user) {
       repository(name: $repo) {
      ...RepoParts
       }
    }
  }

We can however make an alternate api for this that would allow composition,

const reposCountQuery = await gql.runQuery({
  query: gql.query`query MyQuery { ... }`,
  variables: vars,
});

Which is just a shorthand (or longhand) for

const reposCountQuery = await gql.query`query MyQuery { ... }`.run(vars);
yTakkar commented 5 years ago

Having an alternate api for it would be a great idea. :+1:

yTakkar commented 5 years ago

We're gonna write in TS na? If yes: :tada: :man_dancing: :dancer: :tada: :man_dancing: :dancer: :tada: :man_dancing: :dancer: :tada: :man_dancing: :dancer:

yTakkar commented 5 years ago

Let's see what @rakshans1 has to say

phenax commented 5 years ago

@yTakkar, yep TS. So that we dont have to write the typings seperately.

rakshans1 commented 5 years ago

@phenax @yTakkar what is the server implementation for library? Is it to make api calls from server to graph endpoint. In case of SSR?

phenax commented 5 years ago

@rakshans1. Yes. And actually instead of exposing the fetch standard api, we can expose a makeRequest which will get a request object and that way the user can decide how to normalize and make the call with the actual request. Like that, it doesn't necessarily have to be an api call. It can also be an internal call to the graphql function to run a query.

phenax commented 5 years ago

@melwyn95 what do you think about this?

phenax commented 5 years ago

@rakshans1, the "Server library too maybe?" point from the readme refers to a possible wrapper around graphql for node for handling graphql requests. But that's not in the scope right now.

melwyn95 commented 5 years ago

@phenax I have a very beginner knowledge about GraphQL so I cant comment much about the api right now, but anyways I went through the sketch it looks good, I'll do a little more research and comment later.

phenax commented 5 years ago

@melwyn95 @yTakkar @rakshans1, I've pushed a build system with some types based on the graphql spec.

If we all agree with the given api sketch, we should start implementing it. We can refactor it and change the api as we go along. So we can start writing the test cases and the implementation of the core library (query, mutation, memory cache, fragments, etc).

But before that, the first thing we need is...... A FUCKING NAME! So start suggesting names guys!

melwyn95 commented 5 years ago

can we name it Rover after Mars rover ??

phenax commented 5 years ago

Taken. :( https://www.npmjs.com/package/rover

Unless you have a more creative variation for it.

melwyn95 commented 5 years ago

@phenax, gql.query, gql.fragment will return string versions of the Query after interpolation or some internal object representation?

stargazerCountQuery.run(vars);

phenax commented 5 years ago

@melwyn95, query, mutation and subscription methods will return objects with the interface Query, Mutation and Subscription respectively (Not sure what the common interface should be called. Maybe just Operation or OperationType). fragment method will return an object with the interface Fragment. Fragment won't have a run method on it as it can only be used inside queries/mutations/subscriptions. We will extract the fragment code and prepend it to the actual query while interpolating.

yTakkar commented 5 years ago

Can we name it gqler?

yTakkar commented 5 years ago

If we've agreed on the code API implementation, then I think we should move forward. @phenax @rakshans1 @melwyn95

melwyn95 commented 5 years ago

Can we name it gqler?

How about gQuery

phenax commented 5 years ago

Can we name it gqler?

How about gQuery

@melwyn95, you'll make me cry now. Fortunately, that name is already taken -> https://www.npmjs.com/package/gquery

phenax commented 5 years ago

@melwyn95 @yTakkar @rakshans1 I've added some code for the core api shape. I think we are good to start working on some features. I've added a couple of issues, lets discuss who wants to work on what. We will have to take up the wrapper apis later as it depends on the core api.

melwyn95 commented 5 years ago

@phenax I would like to work on fragments wala issue

phenax commented 5 years ago

@melwyn95 Go right ahead. Assigned it to you.