amber icon indicating copy to clipboard operation
amber copied to clipboard

Feature: GraphQL Support

Open AndiLavera opened this issue 4 years ago • 10 comments

Hi, I saw #265 and tried it out. I have a working implementation that i think it is quite nice but i am open to feedback.

I wanted to create PR that adds some command/template that would install GraphQL to an amber project. The PR itself would add no new dependencies to amber. The command would probably be amber g graphql. I am not aware of any flags/options yet.

What would happen once the command is run:

Amber files that would be edited: shard.yml:

  graphql-crystal:  
    github: ziprandom/graphql-crystal  
    branch: master  

application.cr

# below models
require "graphql-crystal"
require "../src/graphql/queries/*"
require "../src/graphql/mutations/*"
require "../src/graphql/schema"
require "../src/graphql/*"
# above controllers

Files that would be added:

- src
    |
    - graphql
    |
      - queries   # holds all GraphQL queries with query_type consolidating the modules
         |
         - query_type.cr
         - *{model}_queries.cr*   # Like user_queries.cr
         |
      - mutations   # holds all mutations with mutation_type consolidating the modules
         |
         - mutation_type.cr
         - *{model}_mutations.cr*   # Like user_mutations.cr
      - schema.cr   # GraphQL schema

Dev's using the framework would then specify a route & controller add include the query, mutation & schema like so:

class GraphQLController < ApplicationController
  include QueryType
  include MutationType
  include Schema

  def index
    schema.execute(graphql_params["query"])
  end

  private def graphql_params
    params.validation do
      required :query
    end
  end
end

Hiccups, side effects & things you should know and should be discussed:

  1. I would prefer the code in application.cr to be in an initializer but the GraphQL controller was throwing an errors because QueryType, MutationType & Schema modules are not defined.

  2. As per the graphql_crystal documentation, you would have to add fields that are queriable to your model like so:

class Post < Granite::Base
  connection mysql
  table posts # Name of the table to use for the model, defaults to class name snake cased

  column id : Int64, primary: true # Primary key, defaults to AUTO INCREMENT
  column name : String? # Nilable field
  column body : String # Not nil field

  # GraphQL
  # `id` would not be queriable
  include GraphQL::ObjectType
  field :name
  field :body
end
  1. I am not currently suggesting adding a command to generate the base for a query file like we do with models (amber g model User email:string). If we did add this, you could essentially run amber g model User email:string, amber g graphql-query User email:string. I am thinking the second command could insert the fields into the model & generate a base query file & add the type to the schema. However, i am not too keen on this. I think it would be cool it hook into the generating model command and adding these graphql files if a certain flag/yml setting is set similar to rails api mode.

Let me know what you think. The graphql shard can be found here

AndiLavera avatar Mar 28 '20 03:03 AndiLavera

I could see adding a new attribute to the column definition to define which fields are queryable.

column name : String?, graphql: true

drujensen avatar Mar 28 '20 14:03 drujensen

That would be a nice addition. I can take a look at granite after making a PR.

A concern has come up though. The author of the shard is MIA from all of github. Contributions to anything are null for 4 months. Furthermore, he isn't even available for issues or releases.

Idk what to do. I plan on forking it in the future and adding some features but no clue when that will be. Maybe I'll pick this idea up when/if that happens.

AndiLavera avatar Mar 30 '20 22:03 AndiLavera

I think is a great addition @andrewc910 feel free to propose a PR. We can all collaborate. There is support for custom generators in amber you might want to take a look at that first. Let us know where we can help.

eliasjpr avatar Apr 06 '20 02:04 eliasjpr

@eliasjpr Can you point me in the direction of those generators? I can only find recipes. I don't think this would be a good recipe as it's not really a 'base' per say. Like amber with react is a base. Maybe if you were planning only an api amber project. I thought some way to install graphql at any life cycle would be nice.

None the less, i am not fully taking over the graphql project but i am currently updating it, adding some tests & doing some clean ups so ameba stop yelling at me. If the shard creator comes back, i would request merging mine into his master branch. At the very least, i can create a simple tutorial on the readme for integrating with amber.

AndiLavera avatar Apr 09 '20 21:04 AndiLavera

This would be epic! One could always look to projects like Prisma for inspiration.

itsezc avatar May 05 '20 10:05 itsezc

@andrewc910 look into this PR #1197 which will allow this type of integration much smoother. You can work with @damianham for questions in regards the plugin approach.

I am very much looking forward to see in the next release of amber :)

eliasjpr avatar May 27 '20 14:05 eliasjpr

@andrewc910 @eliasjpr yes I think the plugin approach is the best way to add features like GraphQL support to Amber projects. PR #1197 makes minimal changes to existing code other than adding to application.cr to load plugins, a shard for modular rendering and refactors recipe_fetcher.cr to DRY up the code. Using the plugin approach, I don't think any changes would be required to grantite or any other base code if we could add a generate subcommand to the plugin command so that plugins could add their own generators and take arguments appropriate to the generator, e.g. given a plugin called graphql

$ amber plugin generate graphql query user **args**
$ amber plugin generate graphql mutation user **args**

A big advantage of the generator being in the plugin is that it would keep the Amber base code lean. Of course this depends on PR #1197 being approved and merged into the Amber codebase. As for the issue of the original shard owner MIA - I suggest to fork the shard into https://github.com/ambercommunity where we can all collectively contribute and to avoid this problem in future.

damianham avatar May 27 '20 15:05 damianham

Hey guys, thanks for the updates. Been pretty busy with work since covid but in a few weeks my schedule should clear up. Still super interested in this.

Glad the plug-in support is making progress. I definitely support the plug-in approach over integrating graphql into Amber/granite.

The shard author merged by crystal version update a few weeks ago. I forked the repo and did quite a bit of code clean up. I'll try and contact the author to gauge his interest. If he is still active, the plug-in will be based off that. If he has little to no interest, I'll move my fork into the Amber community repo after I finish cleaning it up a bit.

AndiLavera avatar May 27 '20 15:05 AndiLavera

@awcrotwell any updates on your fork? I'd love to use Amber the only thing holding me back right now is a lack of support for GraphQL.

itsezc avatar Apr 02 '21 10:04 itsezc

Any update on graphql?

atik7 avatar Apr 26 '21 09:04 atik7