aerogear / offix

GraphQL Offline Client and Server
https://offix.dev
Apache License 2.0
758 stars 45 forks source link

Schema Support for DataStore? #505

Closed Eunovo closed 4 years ago

Eunovo commented 4 years ago

To save entities to the IndexedDB, we need to create it's store first. There are two approaches we could take

  1. We can create all stores that don't exist on application start
  2. We can create the store when an entity is about to be saved to a non-existent store

Currently, I'm using the First approach. I do this by processing the schema and deleting and creating stores accordingly The problem here is that, the schema can't be read from file and process on the client at runtime. A solution to this will be to get the schema from the server using an Introspection Query. But, currently, we do not need the whole schema, we just want to know the stores to create. Which leads me to think that there might be a better way.

If we take approach 2, how do we know when to delete stores?

Also, as you may have noticed, the other lib has Model objects for entities. We also need this because we need to know all the properties of a model for filtering see how I'm doing this on next branch. To avoid this, we could change the approach we take for filtering(I'm thinking something similar to Graphback's approach) so we don't need to know a model's properties.

@wtrocki @kingsleyzissou

Eunovo commented 4 years ago

@kingsleyzissou We were discussing the possibility of importing the graphql file. I was saying that we can't import from outside the client src directory. The model in Datasync-starter as an example is in root/model, the client src is root/client/src. Trying to import the model file would result in an error.

kingsleyzissou commented 4 years ago

Yeah, good point. It would be quite a limiting approach.

Eunovo commented 4 years ago

Following today's discussion, I'll define a schema config format.

So, should it be a declarative JSON like

{
    version: 1,
    models: [
        { ...taskModelDef }
    ]
}

Or should it be a code file like this

export const Task = new Model(modelName, {
    title: string
});

export models = [Task];

I can easily say it's easier to import individual models(they are needed to perform CRUD operations) from the code file.

WDYT? @wtrocki @kingsleyzissou

kingsleyzissou commented 4 years ago

I like the syntax of the second option. Nice and clean, but the first approach may be more flexible.

Just a quick question with regard to the models. Would they all be on the same version all the time, or is there a chance one model could be on a different version than another?

Eunovo commented 4 years ago

@kingsleyzissou the version I refer to is a schema version number. I use it to determine when to update the client dB schema

kingsleyzissou commented 4 years ago

Okay cool. Yeah. In terms of config the json is probably more flexible. Your you could use a plain JavaScript object in a js file.

Let’s see what @wtrocki says

wtrocki commented 4 years ago

1) Do we need to have column names for models? 2) Should version be done additionally per model to know what changed?

wtrocki commented 4 years ago

JSON will be easier to generate and we can provide schema for it.

wtrocki commented 4 years ago

How do you see version migration happening?

Eunovo commented 4 years ago
  1. Do we need to have column names for models?

I need them defined in a Model object for predicate generation.

  1. Should version be done additionally per model to know what changed?

I don't need to update the db schema if model fields change @wtrocki

Eunovo commented 4 years ago

How do you see version migration happening?

@wtrocki So, imagine pushing a new version of your app where new models have been added and some have been removed. You bump the version number to trigger a db upgrade on client device db

Eunovo commented 4 years ago

Okay cool. Yeah. In terms of config the json is probably more flexible. Your you could use a plain JavaScript object in a js file.

The problem is that, the models are defined in a list. We will still need to write js code to filter the list when selecting a single model

wtrocki commented 4 years ago

Real problem here is to give proper API for users. We have learned the lesson before.

Precreation of the models makes sense if we want to:

We want all do this.

I think we should talk first how end user API will look like? How we want to consume this - hooks Vs context provider? And how generic this should be to support Vue/RN/Angular etc. It should be generic first.

If I need to pass around some models thru react components then this is going to be bad API.

Do we want dataStoreContext().User.create() or useUser().create() or something like that.

As for initialization knowing the data structure upfront is essential - with typescript interface defining it.

Later we can add extra flags to some models to subscribe to create, pull for diffs after becoming online etc. All of that based on single well defined interface.

As for not knowing about fields. I would see if we can use codegen plugin later.

Also we need to talk how we going to make operation binding possible - so when I do create - what kind of GraphQL query gets send - and how this query is bound to this.

wtrocki commented 4 years ago

As for the replication it will be worth to spike CRUD data replication handlers based on the GraphQLcrud.org as separate issue

wtrocki commented 4 years ago

To be clear I do not want anyone to answer all of those questions..I will try to cover top level API and design after some research on my side and get back to you tommorow with some thoughts.

My target is to make datastore pluggable while it should work out of the box with graphback generated client side queries and have strong typescript typings. I do not want to involve multiple cli tools or rely on some specific use case - like this will work with GraphQLcodegen only - this might involve moving client side generation out of the graphback etc.

Eunovo commented 4 years ago

@wtrocki What I mean when I say "import model for querying" is this

import { User } from './models';
import { query } from 'offix-datastore';

const results = await query(User, (u) => u.name('eq', 'Test'));

I don't intend to pass models through context. For hooks, an API like this meets the generic requirement

const { data, error, isloading, query } = useQuery(User);
query((c) => c.name('eq', 'Test'));
wtrocki commented 4 years ago
import { User } from './models';
import { query } from 'offix-datastore';

const results = await query(User, (u) => u.name('eq', 'Test'));

What I do not like in this API is that it is not going to be easy to build type-safety on it unless using some conditional types

import { User } from './models';
import { query } from 'offix-datastore';

const results = await query<User>((u: user) => u.name('eq', 'Test'));

but this looks ugly. I believe that the biggest challenges for this project would be:

1) Performance If we decompose relationships now we will need to issue client side queries to the database. In apollo cache approach items are linked to the specific queries making them available without such query

2) Binding to GraphQL queries to replicate/fetch data

We can base on graphql CRUD but this will mean that we will need to take out all graphback plugins and replicate them in offix somewhow

3) TypeSafety for ts API

People can use Graphql-code-generator but it is not clear how our API will be able to reuse or link to those types etc.

4) Tell me :)

Eunovo commented 4 years ago

If we decompose relationships now we will need to issue client side queries to the database.

I don't quite understand. Do you mean that we will need to issue more queries to fetch relationships? or we will need to issue nested graphql queries?

Eunovo commented 4 years ago
import { User } from './models';
import { query } from 'offix-datastore';

const results = await query(User, (u) => u.name('eq', 'Test'));

What I do not like in this API is that it is not going to be easy to build type-safety on it unless using some conditional types

import { User } from './models';
import { query } from 'offix-datastore';

const results = await query<User>((u: user) => u.name('eq', 'Test'));

but this looks ugly. I believe that the biggest challenges for this project would be:

The alternative would be to have something like User.query(): User This is new API reinforces my idea to define a model as an instance instead of a JSON config.

  1. TypeSafety for ts API

People can use Graphql-code-generator but it is not clear how our API will be able to reuse or link to those types etc.

What if the result from User.query matches the interface of the generated types? We could set a model to use a type like this const UserModel = new Model<User>() Where User above could be a generated type

WDYT? @wtrocki @kingsleyzissou

kingsleyzissou commented 4 years ago

I much prefer the User.query() api. I believe Wojciech feels the same, but we will wait to hear what he says.

In terms of the config, I'm not set on any method to achieve this, I would say go for the approach that works better to achieve this.

@wtrocki what are your thoughts?

wtrocki commented 4 years ago

@kingsleyzissou no need to wait for me. Your opinion matters even more here.

The alternative would be to have something like

I think this is sensible, however how we going to deal with relationships? Are we assuming that datastore is flat structure?

In GraphQL when we want to fetch users and their comments we can issue nested query and get all the data. How DataStore API call will look like?

would it be something like:

Users.find(1)
Comments.find(where:{userId: 3})

One thing about User do you see us using codegen to generate this class. Would devs need to created it?

Eunovo commented 4 years ago

I think this is sensible, however how we going to deal with relationships? Are we assuming that datastore is flat structure?

In GraphQL when we want to fetch users and their comments we can issue nested query and get all the data. How DataStore API call will look like?

would it be something like:

Users.find(1)
Comments.find(where:{userId: 3})

This is what I had in mind. @kingsleyzissou any comment?

One thing about User do you see us using codegen to generate this class. Would devs need to created it?

Devs can create it if they want to. See the IUser interface created for a mongoose model here. @wtrocki

kingsleyzissou commented 4 years ago

Yeah I like the api, nice and clean. My only concern is that we might have to provide loads of different query options and use cases, which is impossible.

We would need a way to provide a custom query, but at the same time, since we aren't storing the queries, it's not trivial to implement graphql queries to query the datastore. We could try implement that, but we would be back in cache update hell.

As @wtrocki suggests, we may have to do more advanced queries over the network and use the datastore as the single source of truth. And use the scheduler and offline store to manage the mutations that may have failed due to going offline.

kingsleyzissou commented 4 years ago

Thinking a little bit more about this though. Querying relationships might still be difficult and would possibly have to be split into multiple queries.

Eunovo commented 4 years ago

@wtrocki I think we can close this

wtrocki commented 4 years ago

We need follow up for relationships queries