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

Reconsider default GraphQL APIs? #1859

Open
SachaG opened this issue Jan 29, 2018 · 26 comments
Open

Reconsider default GraphQL APIs? #1859

SachaG opened this issue Jan 29, 2018 · 26 comments

Comments

@SachaG
Copy link
Contributor

SachaG commented Jan 29, 2018

This is more of a long-term thing, but it might be worth reconsidering the signature of Vulcan's internal GraphQL APIs (i.e. list, single, total, new, edit, remove, and now upsert).

For example, is there some kind of "standard" GraphQL way to define these functions? Could we make them compatible with Graphcool or other GraphQL back-ends? How do other back-ends handle things like pagination and deleting fields?

One of Vulcan's strengths is that it's full-stack, meaning that we could change this transparently for the end users (just change the default resolvers and withList at the same time, for example). So I thought it was worth it opening up a discussion, even if we end up not changing anything.

@SachaG
Copy link
Contributor Author

SachaG commented Jan 29, 2018

For example this is what Prisma's generated mutations schema looks like:

https://gist.github.com/gc-codesnippets/f302c104f2806f9e13f41d909e07d82d#file-prisma-graphql-L25-L28

@justinr1234
Copy link
Member

@SachaG
Copy link
Contributor Author

SachaG commented Jan 29, 2018

One thing that's not super clear for me with their update mutation is how you delete fields. I borrowed the set/unset thing from Mongo for Vulcan because it makes it obvious which fields are being deleted vs being just set to empty values.

@justinr1234
Copy link
Member

Likely just a wholesale replacement of document

@SachaG
Copy link
Contributor Author

SachaG commented Jan 31, 2018

@sedubois
Copy link

sedubois commented Feb 5, 2018

I just heard about VulcanJS so apologies if this is a silly question, but is it possible to use Vulcan on top of one's own GraphQL API generated using Prisma? I would love to give it a try if that's possible.

@SachaG
Copy link
Contributor Author

SachaG commented Feb 5, 2018

At this time not really*, but we're discussing how to make that possible.

  • well technically you could, but you'd lose most of the advantages of using Vulcan in the first place.

@sedubois
Copy link

sedubois commented Feb 5, 2018

🙏😊 I think it would be a huge feature, would allow actually benefiting from GraphQL’s universal nature.

@sorenbs
Copy link

sorenbs commented Feb 5, 2018

Would be super cool to use something like graphql-binding to blend in external GraphQL APIs into the Vulcan API.

At Prisma we are currently working with various database vendors to create official specifications for GraphQL API flavours for different kinds of databases including relational, document, search, k/v, and graph databases.

Prismas current API is the foundation for the specification for relational databases.

Example

The SDL schema:

type User {
  id: ID! @unique
  name: String!
}

type Movie {
  id: ID! @unique
  createdAt: DateTime!
  user: User
  userId: ID
  name: String
  year: String
  review: String
}

generates the following API:

API Spec

The full schema spec is available at https://github.com/graphcool/prisma/issues/1340

One of the most powerful features in the relational spec is nested mutations. It is documented at https://github.com/graphcool/prisma/issues/1280

As Vulcan is based on Mongo, you should probably not adopt the current relational Prisma API Spec directly. We will start fledging out the mongo spec soon in https://github.com/graphcool/prisma/issues/1643. It would be great to collaborate on this!

Questions

Sascha asked how Prisma handles setting a field to null versus setting it to the empty value. At least that's how I interpreted the question :-D

We use the null value (introduced in October 2016) to mean set the value to null. As Vulcan is based on Mongo you actually have 3 different options:

  1. set the field to the empty value (ie "" for a string)
  2. set the field to null (ie null)
  3. remove the field

I'm not entirely sure the distinction between 2 and 3 is important enough to justify a more complex API than just using the null value to mean one of the two.

@SachaG
Copy link
Contributor Author

SachaG commented Feb 6, 2018

Yes I think we can decide that setting the field to null means removing it, it's a reasonable convention and wouldn't require a big change anyway.

I guess I just need to set aside half a day to make a proof-of-concept Vulcan+Prisma app now. Unless somebody else wants to work on this?

@sorenbs
Copy link

sorenbs commented Feb 6, 2018

That's great!

I'll get the spec for Prismas MongoDB API going and ask around for anyone interested in contributing to Vulcan/Prisma integration.

@SachaG maybe you can give a few pointers to how someone would go about implementing that?

@SachaG
Copy link
Contributor Author

SachaG commented Feb 7, 2018

Sure, out of the box Vulcan has a set of three default resolvers (single, list, total) and three default mutations (new, edit, remove) on the server; and five corresponding HoCs on the client.

So step 1 would be changing all these to work with the same API as Prisma.

Regarding the Mongo thing though, I think we want the GraphQL API to be database-agnostic, right? So the fact that Vulcan uses Mongo currently shouldn't really matter as far as this issue is concerned?

@SachaG
Copy link
Contributor Author

SachaG commented Feb 7, 2018

Also is there a Graphcool project I could start playing with and entering data into? Or is there a way to open that endpoint in the Graphcool console somehow?

@SachaG
Copy link
Contributor Author

SachaG commented Feb 7, 2018

Oh I guess one thing I overlooked is how we manage accounts… I'm guessing Prisma and Vulcan's user models will be pretty different, which could be a problem. Do you have any documentation on that?

@sorenbs
Copy link

sorenbs commented Feb 11, 2018

@SachaG - The way we think about the GraphQL APIs for different databases is that each database has different strengths and capabilities that should be exposed in the API

My hope is that we can draw a venn diagram of all databases and find a fairly large overlap that can form a common core that is used by all the database APIs while still allowing each database API to include features unique to their underlying datastore. Does that vision make sense to you?

@sorenbs
Copy link

sorenbs commented Feb 11, 2018

The live endpoint linked above is unsecured and publicly available, so you can go ahead and add data directly in the playground.

Alternatively you can install the prisma CLI and have your own public endpoint running in less than 5 minutes so you can also change your schema.

Prisma is completely agnostic to how you structure accounts, so that can be set up however works best for Vulcan. We can jump on a call to chat about it if that would be helpful.

@SachaG
Copy link
Contributor Author

SachaG commented Feb 12, 2018

My hope is that we can draw a venn diagram of all databases and find a fairly large overlap that can form a common core that is used by all the database APIs while still allowing each database API to include features unique to their underlying datastore. Does that vision make sense to you?

Makes total sense. My approach with Vulcan would probably be to focus more on that overlapping core, but provide an "escape hatch" to access the rest of the database API.

And yeah we should definitely jump on a call. I'll message you on Slack.

@SachaG
Copy link
Contributor Author

SachaG commented Feb 12, 2018

After thinking about it some more, the one thing I'm not sold on is the movie/movies thing.

For one thing, I don't love the automatic pluralization since it can go wrong and also doesn't work well with other languages. And it's just one more extra dependency.

But also, you now have both a Movie type and a movie query field, which just seems like a potential source of confusion to me.

I'm not in love with Vulcan's current MoviesList and MoviesSingle, but maybe we could find something in that direction? MovieList and MovieDocument maybe?

@SachaG
Copy link
Contributor Author

SachaG commented Feb 14, 2018

OK so I think AppSync's API can be a good starting point for standardization:

Movie

createMovie
updateMovie
deleteMovie

getMovie
listMovie

@jacobpdq
Copy link

This sounds great. I was going to initally use AppSync until I found the Vulcan framework.

fwiw, the model pluralization also throws me off. I'm fine with naming my collections MovieCollection

@SachaG
Copy link
Contributor Author

SachaG commented Feb 21, 2018

fwiw, the model pluralization also throws me off. I'm fine with naming my collections MovieCollection

The reason it works the way it does is because collections came first, so we just reused the old Meteor/Mongo naming (Movies, Posts, Comments, etc.). I'm not sure if we should change it or not because unlike resolvers, you do have to type out collection/model names a lot throughout your code?

@Michael-J-Montgomery
Copy link

Michael-J-Montgomery commented Mar 20, 2018

Does anyone know the status of Vulcan's support of GraphQL Binding? So we're on the same page, I'll recap my understanding of:

How this could be done (from Prisma's approach)

  • Use SuperGraph framework as an API Gateway on top of existing GraphQL endpoint.
  • Use graphql-cli-prepare to bundle and generate bindings

Issues Vulcan's working on to make it happen (from Github threads)

  • Possibly use merge schema objects using either; GraphQL Static Binding, Merge Graphql Schemas, Apollo mergeSchema function, or Graphql Loader.

I see @SachaG did make progress on some internal issues that could support GraphQL Bindings through complex/nested schemas in the Vulcan-Starter (nested fields, arrays of objects, Stepped form, Facultative fields, FormComponent, and semi-mandatory field)

Thanks!

@SachaG
Copy link
Contributor Author

SachaG commented Apr 17, 2018

@SachaG
Copy link
Contributor Author

SachaG commented Jun 4, 2018

@SachaG
Copy link
Contributor Author

SachaG commented Jun 5, 2018

Here's an example of the schema generated for the Simple example:

https://gist.github.com/SachaG/796a9a9acb6db89b24edc6160276c677

@stale
Copy link

stale bot commented Nov 23, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants