gramps-graphql / gramps-express Goto Github PK

Running yarn live-data still shows mock data. Rendering needs to respect both NODE_ENV === 'production' and GRAMPS_MODE === 'live'.

Add API docs and an intro tutorial for the CLI.

Right now the CLI still calls local data sources "external" data sources, which is weird and confusing.

Also, it links to internal IBM docs, which is probably not all that helpful to most people. So maybe fix that. We'll also need to move the CLI docs into the public docs for that to be possible, so #35 is a blocker for this.

TODO found

https://codeclimate.com/github/gramps-graphql/gramps-express/docs/src/css/block/columns-3.css#issue_59e3f4cbd1822b00010000ab

Missing file extension for "express"

https://codeclimate.com/github/gramps-graphql/gramps-express/src/dev/app.js#issue_59e347f016cb36000100001d

It turns out the IMDB API is pretty unreliable, which is a bummer. This makes it a bad choice for an example data source.

We should revamp the tutorial to use a more reliable API. Requirements are:

• No API key required — we want people to immediately start coding, not have to sign up for API keys
• Reliable — should be up more often than not
• Relatively simple — if we can find a small API with one method, that would actually be the best option. A complete example that doesn’t involve a ton of copy-pasting would make the tutorial much more approachable than having to click through to GitHub for the rest of the code and/or copying hundreds of lines of code to get a proof-of-concept running

I'm currently leaning toward the xkcd API, which is small, reliable, and familiar to many developers.

This should point to the GrAMPS docs or be removed altogether since there aren't specific docs available for this error right now.

The CLI currently doesn't have any tests written. We'll probably need a light refactor to make it easier to test.

The site is showing its enterprise roots and only works on desktops.

The left nav should probably collapse to a dropdown or hamburger or something.

@jlengstorf I really enjoyed your talk today at GraphQL Summit and having the opportunity to speak with you afterwards. Thank you!

(sorry, this ended up being longer than I expected)

I've taken some time to grok the gist of how data-sources are created and how gramps-express consumes them. As I understand it a data source provides four values.

Existing

1. context: String (used to set property name on each queries context)
2. model: Model (added to each queries context)
3. schema: String (text of the Schema)
4. resolvers Object (object with queryResolvers, mutationResolvers, dataResolvers, mockResolvers, and someday subscriptionResolvers)

This took some time to wrap my mind around. I think this is because these values are different than what makeExecutableSchema and addMockFunctionsToSchema expect.

Proposed

I believe that the essence of a data source is more clearly captured with the following four values (which can be passed almost directly into makeExecutableSchema and addMockFunctionsToSchema (and query context).

1. typeDefs: String (text of the Schema)
2. resolvers: Object (the exact object that would be passed to makeExecutableSchema in a standard apollo-server, including Query, Mutation, and Subscription)
3. mocks: Object (the exact object that would be passed to addMockFunctionsToSchema)
4. context: Function (same as function passed directly into graphqlExpress

I believe the Model + Connector pattern you've created is one way to wire up the context and resolvers. Every graphql developer should be familiar with creating typeDefs, resolvers, and mocks (they do it every day!)

Specifics (implementation details)

A new combineResolvers would look something like this:

import merge from 'lodash.merge';
// simply deep merge the objects
export const combineResolvers = (sources, initial = {}) => sources.reduce(merge, initial)

a new getSchema would look like this:

export const getSchema = ({ sources, logger, options }) =>
  makeExecutableSchema({
    // Combine all the schema definitions into an array.
    typeDefs: [rootSchema, ...sources.map(source => source.typeDefs)],
    resolvers: combineResolvers(sources.map(source => source.resolvers), {
        Query: {grampsVersion: () => pkg.version},
        Mutation: {grampsPing: () => 'GET OFF MY LAWN'}
      }
    },
    logger,
    ...options,
  });

export const addMockFunctions = ({ sources, schema, options }) => {
  addMockFunctionsToSchema({
    mocks: combineResolvers(sources.map(source => source.mocks)),
    schema,
    ...options,
  });

Finally, context is added like so: here

return (req, res, next) => {
    const context = sources
      .map(source => source.context)
      .reduce((combined, fn) => merge(combined, fn ? fn(req) : {}), {});

    req.gramps = {
      schema,
      context,
      formatError: formatError(logger),
      ...apolloOptions.graphqlExpress,
    };

    next();
  };

Missing file extension for "express"

https://codeclimate.com/github/gramps-graphql/gramps-express/src/dev/app.js#issue_59e347f016cb36000100001d

Write up a step-by-step tutorial for creating a GrAMPS data source.

Add all-contributors and run it in the Travis process so we're giving contributors proper credit.

In looking at the various Apollo server implementations, the config object (GraphQLOptions) appears to be the same in all situations.

We need to test that, but if that's true, there's really no need to use this middleware. Instead, the API can be simplified in a universally applicable way to:

import gramps from '@gramps/gramps';

const getGraphQLOptions = gramps({ dataSources: [] });

In Express/Connect, we would use:

app.use('/graphql', bodyParser.json());
app.use('/graphql', graphqlConnect(getGraphQLOptions));

In Hapi:

server.register({
  register: graphqlHapi,
  options: {
    path: '/graphql',
    graphqlOptions: getGraphQLOptions,
  },
});

Koa:

app.use(koaBody());
router.post('/graphql', graphqlKoa(getGraphQLOptions));
app.use(router.routes());

Once @gramps/gramps hits a stable release, let's look at deprecating this repo altogether.

The devDependency prettier was updated from 1.14.2 to 1.14.3.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

prettier is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Version 10 of Node.js (code name Dubnium) has been released! 🎊

To see what happens to your code in Node.js 10, Greenkeeper has created a branch with the following changes:

• Added the new Node.js version to your .travis.yml

If you’re interested in upgrading this repo to Node.js 10, you can open a PR with these changes. Please note that this issue is just intended as a friendly reminder and the PR as a possible starting point for getting your code running on Node.js 10.

Your Greenkeeper Bot 🌴

Whenever an error happens in a query, the log is correct on the server, but shows up null on the client side.

Docs for getting a GrAMPS-powered GraphQL server up and running in ~5 minutes. Should start with installing packages and end with a successful query in GraphiQL.

The devDependency nodemon was updated from 1.18.5 to 1.18.6.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

nodemon is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Right now the caching scheme is hard-coded — let's make it easy for a connector to override a method and define their own cache key generation function.

Add middleware to attach a Redis instance to GrAMPS for caching. This will be a new package.

We should have these in place and make it easy to extend them.

The type information isn't showing up for inherited functions and properties.

What needs to change in a data source if data is coming from a database instead of a REST API? Let's figure it out, build a test case, and document it.

The devDependency eslint-plugin-import was updated from 2.18.2 to 2.19.0.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

eslint-plugin-import is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

My team would love to use the GrAMPS architecture, but we're blocked by not having subscription support. It would be great if we could have a basic subscription implementation so data sources can add their own subscriptions.

The error handling process is a little magical/black-boxy — fix that.

TODO found

https://codeclimate.com/github/gramps-graphql/gramps-express/docs/src/css/block/columns-2.css#issue_59e3f4cbd1822b00010000aa

Follow prettier formatting (expected '.' but found '\n').

https://codeclimate.com/github/gramps-graphql/gramps-express/src/helpers/GraphQLConnector.js#issue_59e293e39a0d4a0001000032

Looking at the GraphQLConnector, the dataloader is constructed every time get() is called. This means every HTTP call creates a new dataloader, rather than getting a new dataloader for every graphql request.

It looks like the models and connectors are singletons constructed once, and not for every request. Looking at the GitHunt api, a new model and connector are created for every single request. I'm thinking gramps' behavior should change to match the GitHunt API to enable using dataloaders correctly.

Should the behavior be changed to construct the datasources for each request, rather than treating them as singletons? Otherwise I'm uncertain how per-request caching can occur using dataloaders.

Every method exposed by GrAMPS needs to be documented.