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

Runtime Package Redesign #861

Open
jakeblaxon opened this issue Aug 21, 2020 · 5 comments
Open

Runtime Package Redesign #861

jakeblaxon opened this issue Aug 21, 2020 · 5 comments

Comments

@jakeblaxon
Copy link

Hey guys! First I just want to say I love this project and the ease that it gives us in composing graphql schemas declaratively. Thank you for all of your hard work!

To improve this library even more, I have a couple suggestions for the design of the runtime package to solve a couple issues. Right now, the runtime package is highly coupled to the handler, merger, and transform libraries because it uses types from each library and validates against them. Furthermore, there's no easy way I see to add our own custom handlers/mergers/transforms without adding them to the graphql-mesh monorepo.

These problems can be solved if we treat the runtime package as a core framework library and use dependency injection instead. We can think of handlers, mergers, and transforms as plugins that we use in our yaml file. The runtime package that loads our yaml file has no knowledge of what these actual plugins are, it just knows that they follow a certain contract. An example for this contract could be something like the following:

(options: {schema: GraphQLSchema, config: ...}) => GraphQLSchema | Promise<GraphQLSchema>

where each plugin is given the current schema and returns the transformed schema. The graphql-mesh framework can additionally pass in other configs to the options object that are user-specified in the yaml file, making these plugins declarative and behave similarly to how the standard plugins behave now.

This gives us several advantages:

To illustrate this design, I made a proof of concept MVP, and I've been using it to much success on my own projects. Please reference the wiki and the core package specifically. If you agree with this design, I'm entirely willing to donate this package to graphql-mesh, or help with creating a new one. Just let me know your thoughts.

This is a great library and I'm very excited to help with its development. Thank you once again for this awesome project!

@jakeblaxon jakeblaxon changed the title Runtime package redesign Runtime Package Redesign Aug 21, 2020
@ardatan
Copy link
Owner

ardatan commented Aug 22, 2020

Thank you for bringing those awesome ideas!
Let me give some info from latest changes in GraphQL Mesh. Recently we extracted configuration and module loading related stuff from @graphql-mesh/runtime package because this caused GraphQL Mesh heavily relied on NodeJS environment due to dynamic module loading, file system stuff etc. But we had some issues on non-NodeJS environments which are using Webpack and stuff and all those were breaking this kind of environments. That's why, we seperated those packages. Now @graphql-mesh/config finds and parses the configuration file then loads the necessary packages from node_modules and does some FS stuff independently from @graphql-mesh/runtime. After all these, it passes all those prepared stuff to getMesh of @graphql-mesh/runtime. This seperation is still WIP btw. So the default merger and caching packages aren't coupled with runtime package but config package.

As I see, you have a problem of extending GraphQL Mesh with custom plugins because of the strict validation of configuration package. So we can make it more flexible instead of doing a big refactor. We can allow people to use their custom merger, handler or transform by making that config validation more flexible in case of the usage of an unknown extra package/module/plugin name.

For dynamic schema manipulation, we were already thinking of using hooks which is based on EventEmitter. Typed events like schemaReady: schema, schemaChange: oldSchema, newSchema, onTransform: schema and onMerge: schemas etc can be used and for example express-graphql can serve updated schema without restart in this case.

I am not sure about making all concerns under a single plugin approach because handling sources, transforming schemas, stitching them and stuff are different concerns. We should keep all these seperated in my opinion.

@dotansimha What do you think?

@jakeblaxon
Copy link
Author

jakeblaxon commented Aug 22, 2020

Awesome! I wasn't aware of the new config package, so it looks like that issue with runtime being coupled is resolved. However, the underlying problem is still there, since the config package validation depends on the YamlConfig type, which in turn must get updated each time any of the plugin packages' configs get updated. If we instead passed off the responsibility of validation to each plugin for their respective configs, that would allow each plugin to evolve independently without having to update any of the core packages. The core packages would just expect each plugin to implement a certain plugin interface, and they would truly be decoupled from the plugins since they don't depend on their specifics. This would make the development experience easier and allow outside contributors to add contributions faster, since they would only have to update the one package they're interested in.

We can write our own findAndParseConfig function to prepare the yaml config for the getMesh method. That would allow us to use our custom plugins. The biggest problem with this though is that the process of building a config object for getMesh is not well documented at the moment. It'd be easier if we could instead just pass in the raw yaml config object to getMesh along with a provider that implements a certain interface, such as a loadPlugin(name: string) function that returns a specific plugin implementation by name. This would be easy to implement and give us full control over what plugins the runtime actually uses. This would also give us the advantage of not having to use a different process (like hooks) to provide our own plugins dynamically. That all gets abstracted away by the pluginLoader provider.

If we can just pass in the raw yaml config, then we could also abstract away the concern of loading the yaml config. The runtime package would have a default way to load it, of course, but we can load it ourselves if we need to through any means and just pass it directly to getMesh without any processing. Essentially, everything gets handled with dependency injection and providers, allowing us to replace the default providers that runtime uses with our own. This gives the user complete control, only relying on the runtime package to do the tedious work of actually combining and transforming the schemas according to the yaml config.

Regarding plugins, there would be different plugin types, like mergers and handlers, but allowing them all to implement a single interface would make certain plugins reusable in different contexts (for example, the federation plugin could be a handler, transform, and merger. It would know which action to perform based on a parameter passed in by the runtime). This would allow us to combine handlers/transforms/mergers that are closely related together into one package. Of course, if you only want to implement a transform or a handler, you could write a plugin just for that type.

@ardatan
Copy link
Owner

ardatan commented Sep 6, 2020

#907
Configuration logic has been decoupled completely from runtime package(you can check react example to see how new programmatic API is);
and also you can use your own handlers, transforms and cache impl. in yaml/json config files from now on(new json schema allows the additional properties under sources, transforms and cache fields).

@jakeblaxon
Copy link
Author

jakeblaxon commented Sep 13, 2020

This is great! Thanks for your work on this!

Two other suggestions I was thinking:

  1. How would you feel about allowing nested meshes, i.e. allowing someone to add sources and a merger within a source itself? For example, a possible mesh config could look something like:
merger: stitching
sources:
  - name: "outerSource"
    merger: federation
    sources:
      - name: ...

This would basically allow us to recursively nest the mesh config structure within a source, so that the source config would look something like:

export type MeshResolvedSource<TContext = any> = {
  name: string;
  handler: MeshHandler<TContext>;
  transforms?: MeshTransform[];
} 
| GetMeshOptions;
  1. How would you feel about adding a transform plugin that allows you to add additional typedefs and resolvers, instead of offering that within the config itself? To me, this would have a few advantages:
  • We can add additional typedefs and resolvers to a pre-merged source via a transform, giving us more flexibility to transform our schemas. Right now we can add these at the merge level, but we can't add them to individual sources, where we may want to do so and then apply additional transforms afterwards (like adding a prefix, for example).
  • We can eliminate the concern of dealing with that functionality in the runtime package and delegate that logic to its own plugin package.

Of course, we have the option of writing our own transform to do this, but that would make the current additionalTypedefs and additionalResolvers fields unnecessary, since we could just use that transform on the outer schema instead.

These are both use cases I've encountered so far, and having these capabilities would be helpful. I can also help with implementing one or both of them if you agree. Curious to hear your thoughts!

@ardatan
Copy link
Owner

ardatan commented Sep 15, 2020

Sorry I accidentially closed the issue :)
@jakeblaxon We can have nested meshes in a new handler :) I think it is doable!
But I am not sure about the second one because additional typeDefs and resolvers should be on top of other schemas because it is how graphql-tools's schema stitching works :/
https://www.graphql-tools.com/docs/stitch-schema-extensions

@ardatan ardatan closed this as completed Sep 15, 2020
@ardatan ardatan reopened this Sep 15, 2020
@theguild-bot theguild-bot mentioned this issue Aug 11, 2022
@theguild-bot theguild-bot mentioned this issue Sep 28, 2023
klippx pushed a commit to klippx/graphql-mesh that referenced this issue Oct 9, 2024
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

2 participants