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

About splitting the project into a general adapter and a mongoose. #105

Open
nodkz opened this issue Mar 23, 2016 · 14 comments
Open

About splitting the project into a general adapter and a mongoose. #105

nodkz opened this issue Mar 23, 2016 · 14 comments

Comments

@nodkz
Copy link
Contributor

nodkz commented Mar 23, 2016

@tothandras said: I have started to split the project into a general adapter (that waits a graffiti model as input) and a mongoose specific one.

Did you share somewhere a Proposal of your changes? How it will look like?

It will be great if new version will support:

  • ability extend generated types by additional fields in graphQL manner with custom resolvers
  • support mongoose model virtual fields
  • getOneResolver can be wrapped by DataLoader for performance reasons
  • ability to add GraphQL Interfaces for generated types
@tothandras
Copy link
Contributor

@nodkz I will write a detailed proposal, but basically the Graffiti model would be the connection between the adapters. The general adapter would have a Graffiti model as an input and a query object that has all the getOneResolver, so on functions.. It is then passed down to every resolve function on info.rootValue.

@nodkz
Copy link
Contributor Author

nodkz commented Mar 24, 2016

Hmm, may be very interesting. Need code example.

@nodkz
Copy link
Contributor Author

nodkz commented Mar 24, 2016

Found this comment #64 (comment)

I'd like this idea, and suggest implement it like wrappers:

let Post = mongoose.model('Post');

// current behaviour
const schema = getSchema([Post]);

// add wrapper
const schema = getSchema([withFilesConnection(Post)]);

// add more
const schema = getSchema([withFriendsQueries(withFilesConnection(Post))]);

Reusable wrappers under mongoose models is simple and extendable solution.

@nodkz
Copy link
Contributor Author

nodkz commented Mar 24, 2016

For example virtualFieldsWrapper can solve task of populating a mongoose virtual fields. Mongoose has not define types for virtual fields. So wrapper can solve this problem:

const Post = mongoose.model('Post');
const PostWithVirtual = withVirtualFields(Post, {'name.full': string, 'metaInfo': MetaSchema});

const schema = getSchema([PostWithVirtual]);

Also some hooks may be hidden under wrappers. So wrappers can eliminate extending graffiti options in mongoose schemas.

@nodkz
Copy link
Contributor Author

nodkz commented Mar 24, 2016

Proposal implementation of wrappers: nodkz@0f72a8b

import mongoose, { Schema } from 'mongoose';
import { GraphQLString } from 'graphql/type';
import ExtraFieldsWrapper from 'lib/graffiti-mongoose/src/wrappers/extra-fields';
import VirtualFieldsWrapper from 'lib/graffiti-mongoose/src/wrappers/virtual-fields';

const CvSchema = new Schema(
  {
    name: {
      type: String,
      description: 'Person name',
    },
  },
  {
    toObject: { virtuals: true }, // for population virtual values in resolver
  }
);

CvSchema.virtual('name333').get(function () {
  return '!!!!!!!!' + this._id + '!!!!!!!!';
});

const Cv = mongoose.model('Cv', CvSchema);

let CvWrapped = new ExtraFieldsWrapper(Cv, {
  extraFields: {
    _session: {
      type: GraphQLString,
      resolve(rootValue, args, info) {
        return `This is extra field which returns id: ${rootValue._id}`;
      },
    },
  }
});
CvWrapped = new VirtualFieldsWrapper(CvWrapped, {
  virtualFields: {
    name333: 'String',
  }
});

const Schema = getSchema([CvWrapped]);

@nodkz
Copy link
Contributor Author

nodkz commented Mar 25, 2016

New problem reveal today. I have Cv model, and users can leave Notes to it. But Notes must be available only to those users who have left it.

Cv does not store references to Notes (only Note know about user and cvId). So I want get such query:

query {
  Cv(id: "1") {
     onlyMyNotes: notes {
        text
        createdAt
     }
  }
}

To solve this problem I should:

  • generate GraphQLType for Notes, before I call getSchema
  • add extra field to Cv via my wrapper above ExtraFieldsWrapper and resolve it with filtering by current user from req. At this point I also want use DataLoader for reducing number of queries to DB.

So graffiti does not allow to create gq-types, and use it later for extending other types. After that I may pass needed types to getSchema for root query population. Also I want pass Notes type to mutations, but not for queries.

Right now I will do monkeypatching 😉 of graffity in my wrapper-branch. I think it can give you some new thoughts about splitting the project in right way 😜

@tothandras
Copy link
Contributor

@nodkz Thanks for the very detailed proposal! I really appreciate it! :) I will dig into it more as soon as I have some free time.

@nodkz
Copy link
Contributor Author

nodkz commented Mar 29, 2016

Also I will use DataLoader (nice 30 min walkthrough from Lee Byron) between Mongoose and GraphQL. So need ability to switch getOneResolver (use or not) this great feature, which can speed up querying for some graphs.

Eg. if we want load authors metadata for 100 articles, best choice would be using batchLoading via DataLoader (internally it creates queue of promises and for next process tick combines all needed authors ids and make one batch query to retrieve authors data. After that all promises would be resolved in GQL). Awesome speed up!

@nodkz
Copy link
Contributor Author

nodkz commented Mar 29, 2016

App schema example https://gist.github.com/nodkz/164f410c78d7a8f01a0d
Extending Cv model by field userData in graphQL manner. This field type obtained from another mongoose model UserData, which does not use mongoose references/population with Cv. Also used custom resolve function, which can use DataLoader, but in this example use filtering by userId.

@nodkz
Copy link
Contributor Author

nodkz commented Mar 30, 2016

For React/Relay components I should have ability use GraphQL Interfaces. Some components can be rendered under different types (so it can be done, if we add interface to such types and will use fragment on interfaceName), so to have such ability I create `ExtraInterfacesWrapper.

Commit: nodkz@08aa4a3
Updated gist with app/schema.js: https://gist.github.com/nodkz/164f410c78d7a8f01a0d

@tothandras
Copy link
Contributor

@nodkz Thanks for your effort! Sorry for not being active lately, I will try to keep up with your progress asap.

@sibelius
Copy link
Contributor

sibelius commented Jun 1, 2016

@nodkz any progress on this?

@nodkz
Copy link
Contributor Author

nodkz commented Jun 3, 2016

@sibelius internally I totally rewrite module with different architecture for our project purposes.
But it not ready for OSS yet, cause API is not stable (I don't like that some things can be done via different ways and also I have not enough free time to document it).

@nodkz
Copy link
Contributor Author

nodkz commented Jun 7, 2016

@sibelius @tothandras just started develop new module https://github.com/nodkz/graphql-compose

Also you may see my previous version https://github.com/nodkz/graphql-mongoose which I won't publish to npm (reason - mash code in app).

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

No branches or pull requests

3 participants