Coder Social home page Coder Social logo

Comments (27)

KyleAMathews avatar KyleAMathews commented on May 6, 2024 1

Also I don't use typed objects for passing around data—afaik this is relatively uncommon in the JS world. So other than inspecting field names, I don't have a way to know what type an object is.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

Same, actually - I'm using JSON API so I just branch obj.type. Same thing though.

from graphql-relay-js.

skevy avatar skevy commented on May 6, 2024

I'm passing around immutable.js iterables everywhere in my app, so I have a prepareForGraphQL function that I call wherever I resolve an object. It looks like:

export function prepareForGraphQL(arrayOrObjOrImmutable, typeName, customTransformer) {
  const transformer = (obj) => {
    if (customTransformer) {
      obj = {
        ...obj,
        ...customTransformer(obj)
      };
    }

    return {
      $$typeof: Symbol.for(typeName),
      ...obj
    };
  };

  let arrayOrObj;

  if (Iterable.isIterable(arrayOrObjOrImmutable)) {
    arrayOrObj = arrayOrObjOrImmutable.toJS();
  } else {
    arrayOrObj = arrayOrObjOrImmutable;
  }

  if (Array.isArray(arrayOrObj)) {
    if (arrayOrObj.length === 0) return null;
    return arrayOrObj.map(transformer);
  }

  if (!arrayOrObj) return null;

  return transformer(arrayOrObj);
}

And an isTypeOf function that looks like:

export function isTypeOf(typeName) {
  return (obj) => obj.$$typeof === Symbol.for(typeName);
}

I don't use the second argument of nodeDefinitions, and instead rely on isTypeOf in my GraphQLObjectType definitions (which of course calls my custom isTypeOf function). I actually "register" my types with a registry, so that I don't have to have to deal with any weird circular dep issues. That piece looks like:

const TYPE_REGISTRY = {};

export function registerType(type, resolveById) {
  TYPE_REGISTRY[type.name] = {
    type,
    resolveById
  };
  return type;
}

export const {nodeInterface, nodeField} = nodeDefinitions(
  (globalId, info) => {
    const { type, id } = fromGlobalId(globalId);
    let obj = TYPE_REGISTRY[type] ? TYPE_REGISTRY[type].resolveById(id, info) : null;
    return obj;
  }
);

// and then, in my individual type files

const userType = registerType(
  new GraphQLObjectType({
    name: 'User',
    fields: () => ({...}),
    isTypeOf: isTypeOf('USER'),
    interfaces: () => [nodeInterface]
  }
, userFetcher);

This is not the end-all-be-all way to solve this issue, but I'd thought I'd share for both discussion and to provide insight into one possible way to solve the problem.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

Isn't that less performant because it requires iterating through every candidate type? https://github.com/graphql/graphql-js/blob/f9a287754898faf850101045f7d63000ea1b2159/src/type/definition.js#L584

from graphql-relay-js.

skevy avatar skevy commented on May 6, 2024

Yes. But 1) it's only looping through types that implement the nodeInterface and 2) even if you had like 50 node types (which seems like a lot, even for a large app), would you really ever notice the difference?

But in general, yes. You're right. So it may be worth implementing my own resolver and not rely on isTypeOf.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

Huh? If you have a type registry, don't you trivially have a type resolver that just does a map lookup into the type registry?

This both lets you avoid circular imports if you split up your schema, and gives you an efficient ubiquitous type resolver.

I'd argue this is actually a better pattern than having e.g. a resolveWithType method for abstract types.

from graphql-relay-js.

nickretallack avatar nickretallack commented on May 6, 2024

I'm surprised Relay doesn't do this mapping for you. I mean, you already have to name your GraphQLObjectType. It'd be nice if globalIdField could introspect on that. And then if you could give each GraphQLObjectType that implements nodeInterface some sort of resolver method that takes an ID, nodeDefinitions could introspect on that for its first argument. Not entirely sure how the second argument to nodeDefinitions is used though. Perhaps it could have the same pattern, where you associate a GraphQLObjectType with a JavaScript object type and it introspects that too, but like you said, who knows how you'd want this mapping to work in practice?

I guess you could do all this manually, by building your own registry, but the schema objects are already a registry, so it'd be nice if you could toss in this extra information.

I suppose the reason this isn't automatic is because people may want to use their own ID schemes in place of globalIdField/fromGlobalId.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

After some use, I'm getting enough out of a central node registry anyway that I don't think this is really a relevant ask. Thanks!

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

@taion I'm trying to split up my Types (and Mutations) into their own files. Do you have an example of a schema that has been split up?

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

Just put each of them in their own file, and use thunks for fields.

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

@taion thanks for the quick response.

I'm not familiar with thunks. But I was not under the impression that control flow (or async) was the cause of my troubles (of course, I could be wrong). The error I'm getting, seen here graphql/graphql-js#146, seems to be related to calling the new GraphQLObjectType constructor in the dedicated Type file.

I am stabbing in the dark at how to avoid that. Are thunks the answer to that problem? Since your comment in this issue is just about the only mention of splitting up a GraphQL schema I can find anywhere, I thought maybe you knew of an example.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

You want to do it like this:

// ./src/schema/types/parent.js
import { GraphQLObjectType } from 'graphql';

import ChildType from './ChildType';

export default new GraphQLObjectType({
  name: 'Parent',
  fields: () => ({
    parentId: {
      type: GraphQLString,
      resolve: ({id}) => id,
    },
    child: {
      type: ChildType,
    },
  }),
};

// ./src/schema/types/child.js
import { GraphQLObjectType } from 'graphql';

import ParentType from './ParentType';

export default new GraphQLObjectType({
  name: 'Child',
  fields: () => ({
    parentId: {
      type: GraphQLString,
      resolve: ({id}) => id,
    },
    parent: {
      type: ParentType,
    },
  }),
};

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

Thanks, that makes sense.

If you end up importing schema/types/child.js in more than one parent, isn't that going to be calling the new GraphQLObject constructor repeatedly, triggering an error?

Also, where would nodeInterface be constructed and how would it be assigned as the interface for the ObjectTypes living in separate files?

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

Take a look at https://gist.github.com/taion/d161a58b9f7381d8fa9c.

This lets me just make my Node definitions be e.g.:

// ./src/schema/node.js
import { nodeDefinitions } from 'graphql-relay';

import { idFetcher, typeResolver } from './registry';

export const { nodeInterface, nodeField } = nodeDefinitions(
  idFetcher, typeResolver
);

The item fetching stuff is fairly specific to how my REST backend is laid out, in that I can directly map my GraphQL type names to the REST endpoints most of the time, but the basic idea should be fairly general. To avoid code duplication, I'll mostly use the exported getEndpoint from registry.js to do data fetching in general.

At some point I'll properly open source this as a library once I figure out how to make fully generic, but don't hold your breath there.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

The API assumption I have baked in there is that if I have a type named Widget, the corresponding endpoints on my REST backend will be /api/widgets and /api/widgets/:widgetId.

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

Thanks. I'm definitely out of my depth.

It looks like this addresses issues with nodeDefinitions. But I'm not sure how this avoids the repeated constructor calls issue when a Type is a field on more than one other Type. Does it also address that?

It seems like this graphql-relay-js repo, on its own, doesn't support splitting up a schema. Is that truly missing?

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

There's no repeated constructor call with e.g. the pattern I have above. The object types are only constructed once; it's just the calculation of the field types that's deferred.

In principle you can split up a schema however you want using the patterns currently available in graphql-relay. In practice I think centralizing type resolution and node data fetching like I have in my registry is a much more sane approach for a schema of any meaningful size, but it's hard to set up an abstraction there that's both generic and useful - hence why it's left up to us in userspace.

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

Does this also cover splitting up connection types, edge types, and mutations? I imagine keeping all the connection definitions in one file and splitting up mutations.

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

I've been going whole hog and putting each of my connections in a separate file. Why not, right?

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

Sure, why not. Do you register those connection and edge types and mutations the same way you register object types?

And is this how you end up exporting an object type?

export default registerType(MyType);

Or how does your registry pattern replace this pattern from the examples:

var {nodeInterface, nodeField} = nodeDefinitions(
  (globalId) => {
    var {type, id} = fromGlobalId(globalId);
    if (type === 'Viewer') {
      return getViewer();
    } else if (type === 'User') {
      return getUser(id);
    } else {
      return null;
    }
  },
  (obj) => {
    if (obj instanceof Viewer) {
      return ViewerType;
    } else if (obj instanceof User) {
      return UserType;
    } else {
      return null;
    }
  }
);

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

That's how I export, yes.

I don't register those types because they aren't nodes.

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

Ok, so its safe to just import them as normal modules I guess. graphql-relay doesn't barf when you call connectionDefinitions on the same connection more than once(?)

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

Let's take this off the graphql-relay-js issue tracker - it's not a great medium for this. Try catching someone on the Relay channel on Reactiflux.

from graphql-relay-js.

ryanblakeley avatar ryanblakeley commented on May 6, 2024

Sure thing, thanks a bunch for answering all those questions.

from graphql-relay-js.

kevinSuttle avatar kevinSuttle commented on May 6, 2024

I ended up here trying to track down a solution to an issue with circular dependencies and Interface type resolutions. Purely an issue with graphql-js, I know, as I'm not using Relay in this app, but it seems like this is a very common problem.

If you are forced to make a registry for every Schema, what's the point of Interfaces? That's essentially a registry, conceptually.

Related issues:
graphql/graphql-js#277
#66

See also: @wincent's implementation: https://github.com/wincent/masochist/blob/master/app/src/server/schema/definitions/node.js

from graphql-relay-js.

taion avatar taion commented on May 6, 2024

I've found interfaces useful because they let me write Relay components that only expect an object of the interface type, and only deal with fields there.

from graphql-relay-js.

kevinSuttle avatar kevinSuttle commented on May 6, 2024

@taion Should we tack this discussion onto one of the issues I referenced?

from graphql-relay-js.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.