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

Compatibility with Mirage’s ORM layer? #18

Open
jgwhite opened this issue Apr 17, 2019 · 18 comments
Open

Compatibility with Mirage’s ORM layer? #18

jgwhite opened this issue Apr 17, 2019 · 18 comments
Labels
enhancement New feature or request

Comments

@jgwhite
Copy link
Contributor

jgwhite commented Apr 17, 2019

It seems like this library currently interfaces directly with Mirage’s DB layer, rather than the ORM layer. This causes some compatibility problems for projects that use the ORM layer, particularly in code paths such as filterByParent†.

Interfacing with the ORM layer would potentially simplify loading of associated records and handling of things like non-standard inverse field names.

Is there a reason not to interface with the ORM layer?

Would a PR that switches ember-cli-mirage-graphql over to the ORM layer be valuable?‡


filterByParent filters on someParentField.id on objects from the DB. When using the ORM layer, the relationship is stored in the DB as someParentFieldId.

‡ We’d probably have to take backwards compatibility into account for projects that don’t use the ORM layer yet.

@jneurock
Copy link
Contributor

Thanks for opening this issue! There's no reason not to interface with Mirage's ORM layer. A PR to address this issue would certainly be valuable :)

@jneurock jneurock added the enhancement New feature or request label Apr 17, 2019
@jneurock
Copy link
Contributor

@jgwhite, if you want, you can create a test that fails and send it my way.

@jgwhite
Copy link
Contributor Author

jgwhite commented May 22, 2019

@jneurock thanks. The team is digging back into this in the next few weeks so hopefully that’ll force us to contribute some test cases and/or an implementation.

@jneurock
Copy link
Contributor

jneurock commented Dec 9, 2019

@jgwhite, it seems like this might only work if you use Mirage models. Do you think that's accurate?

In my apps, I don't use any Mirage models, only factories. I don't want to go through the trouble of creating Mirage models since there's a lot of overlap with the GraphQL schema. As a result, I seem to only be able to work with Mirage's DB layer.

I would like to continue not using Mirage models so maybe a better approach here would be to examine the differences at the DB layer when using Mirage models and relationships vs. having no Mirage models at all.

That or the logic could be split for the two cases. One strategy for working with the ORM layer and a fallback for the DB layer, if needed.

Let me know what you think.

@jgwhite
Copy link
Contributor Author

jgwhite commented Dec 9, 2019

it seems like this might only work if you use Mirage models. Do you think that's accurate?

Yes, I think that’s the case.

In my apps, I don't use any Mirage models, only factories. I don't want to go through the trouble of creating Mirage models since there's a lot of overlap with the GraphQL schema. As a result, I seem to only be able to work with Mirage's DB layer.

That makes sense. I know Mirage is able to automatically infer models from Ember Data models. I wonder if a similar approach could be taken with the schema? Perhaps this indicates the need for a new public API in Mirage for registering models?

I would like to continue not using Mirage models so maybe a better approach here would be to examine the differences at the DB layer when using Mirage models and relationships vs. having no Mirage models at all.

Yeah, that sounds like a good strategy, assuming that the way Mirage stores the relationship data at the DB layer is stable.

That or the logic could be split for the two cases. One strategy for working with the ORM layer and a fallback for the DB layer, if needed.

If the above strategy works it would certainly be simpler to maintain.

@jneurock
Copy link
Contributor

jneurock commented Dec 9, 2019

Thanks, @jgwhite.

Yeah, that sounds like a good strategy, assuming that the way Mirage stores the relationship data at the DB layer is stable.

I think this has been stable for a long time but I guess I'll find out...

That makes sense. I know Mirage is able to automatically infer models from Ember Data models. I wonder if a similar approach could be taken with the schema? Perhaps this indicates the need for a new public API in Mirage for registering models?

I think we could create Mirage models from the schema with some work but yeah, how do we make Mirage aware of them? Any thoughts @samselikoff? Is this already possible or would we need a public API?

@samselikoff
Copy link

Could definitely create models from a schema, and yes I think the ORM is still valuable + should be the way a GraphQL lib interacts with Mirage's data layer. The ORM is what lets you pass models into other models while doing server.create, updates inverse relationship foreign keys in the db, and many other things. I think our goal should be to auto-define Mirage's models + relationships from a graphql schema, in the same way we do with Ember Data. I haven't worked with graphql relationships yet so an example to start would be very helpful.

@jneurock
Copy link
Contributor

jneurock commented Dec 9, 2019

Thanks, @samselikoff.

Quick example of a blog post type in a GraphQL schema:

type Post {
  id: ID!
  author: User # belongsTo
  body: String! # scalar
  comments: [Comment] # hasMany
  title: String! # scalar
}

When this gets parsed into JavaScript, we get all kinds of good info. For example, the parsed schema would tell us the comments field has a return type of GraphQLList with a property, ofType, of GraphQLObjectType which has the name "Comment". Plenty of info from which to create Mirage models.

@jneurock
Copy link
Contributor

jneurock commented Dec 9, 2019

I can also make it part of my work on #34 to go ahead and create Mirage models from the schema but is there a way to make them known to Mirage programmatically? I'll try to find out when I get to it.

@samselikoff
Copy link

The programmatic answer would be to get the models/relationships from graphql into some data structure and then pass them into Mirage's config.

Here's what it looks like for a new server:

import { Model, hasMany } from 'miragejs'

new Server({
  models: {
    user: Model.extend({
      comments: hasMany()
    }),

    comment: Model.extend({
      user: belongsTo()
    })
  }
})

but you can "augment" existing mirage server's config by calling config(options) again:

let server = new Server();

// later...

server.config({
  models: {
    user: Model.extend({
      comments: hasMany()
    }),

    comment: Model.extend({
      user: belongsTo()
    })
  }
})

Basic goal would just be to form that models hash using data from graphql schema.

If it helps, we can also expose some method on server like server.registerModel('user', definition), something like this. Believe schema object already has this but if we can start just using config options to start that might be easiest.

@jneurock
Copy link
Contributor

jneurock commented Dec 9, 2019

Thanks for the info! It looks like I can go ahead and try this out: https://github.com/miragejs/miragejs/blob/master/lib/orm/schema.js#L58.

@samselikoff
Copy link

Exactly, nice find 👍

@jneurock
Copy link
Contributor

Well, it's been a long time but I started working on the plan proposed here a few weeks ago and wanted to leave a progress update.

  1. I started by creating a separate GraphQL auto resolver library like I mentioned. I got that mostly working but then realized that I didn't need it. More on that later...
  2. Working with Mirage's ORM layer seems to be working well so far.
  3. Sam created a Mirage GraphQL project so my plan is to retire this add-on in favor of the new library when it's ready. There was no Ember specific code in here anyway.

Some technical details...

The graphql-js library actually provides a lot of what I need, I just needed to learn more about the internals. This allows me to avoid creating a separate GraphQL auto resolve library like I planned.

Additionally, I'm able to pretty easily create Mirage models with proper relationships based on a GraphQL schema. The GraphQL schema, as tested so far, is a good stand-in for defining Mirage models explicitly and it could be that the conventional workflow is to avoid the redundancy of defining Mirage models when using GraphQL.

All but 4 of the test modules from this add-on are currently passing and 2 of the 4 are mutation tests which rely on user-defined resolvers vs. automatic resolution.

Remaining tasks:

  • Get all current, relevant tests green. Some tests are rendered irrelevant by the new library.
  • Expand the GraphQL schema for tests to cover more use cases. There are likely few remaining.
  • Document the new library. I'm not sure how best to do this and will need input from Sam.
  • Determine best way to deprecate this add-on.

Feel free to leave questions about the progress here. Thanks for the patience.

@samselikoff
Copy link

Awesome. Can't wait to see the work.

I mentioned to Rocky in Discord, I'm about to wrap up the REST Tutorial for Mirage, and it would be so great to eventually have a GraphQL version of it that we maintain alongside of it 👍

Will be more than happy to help with the docs/examples side once we get to that point.

@jgwhite , has anything changed on your end in terms of how y'all are using Mirage + GraphQL?

@jgwhite
Copy link
Contributor Author

jgwhite commented Jun 22, 2020

has anything changed on your end in terms of how y'all are using Mirage + GraphQL?

@samselikoff nothing’s changed on our end. Still excited to try out any new ideas in our apps. I know @chadian has been exploring some ideas with GraphQL and Mirage too.

@jneurock
Copy link
Contributor

With our powers combined...

@chadian
Copy link
Contributor

chadian commented Jun 22, 2020

Hello! Yes, I have been toying with the graphql testing ideas the past few months. I had an initial spike in December that I picked up a few months ago and has since come along quite far. The idea I've been working on has a mirage component for "auto resolving" but also even more broadly provides a suite of tools for bootstrapping a graphql handler or resolver maps for testing (ie: getting automated sinon or logging added quickly).

The mirage piece received some updates yesterday (graphql-mocks/graphql-mocks#10) and there's a few other ideas I would like to add later if I have time. I'm currently working on documentation and hoping it'll be ready for people to try in a week or two.

@jneurock
Copy link
Contributor

jneurock commented Jul 5, 2020

It's been a long time coming but almost there... miragejs/graphql#1 for anyone that would like to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants