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

Allow configure mode default + support all transforms on all sources #1930

Open
ntziolis opened this issue Apr 9, 2021 · 1 comment
Open

Comments

@ntziolis
Copy link
Contributor

ntziolis commented Apr 9, 2021

Part 1:
As of right now wrap is default mode for transforms. If a bare mode is supposed to be used it needs to be configured for each transform individually.

We propose adding the ability to override this default:

  • on root level of config
  • and (potentially) on source level config

Part 2:
As of right now not all transforms can be used with graphql sources due to not supporting wrap mode. When applying a bare transform on graphql source there currently is no error thrown the transform simply doesn't get applied. It took us a day or so to figure out what went wrong. While this could be mitigated by throwing an error, not being able to apply all transform to graphql source adds complexity for users to understand and reduces the supported scenarios for graphql mesh.

We propose:

  • any transform should be able to apply on any source (this means all transforms would need to support wrap mode in addition to bare).
  • Should a source not support bare transforms, the source should ignore any mode settings and apply wrap transforms

This reduces the potential for miss configuration as well as simplify the messaging:

  • Any transform can be applied on any level
  • Transforms have 2 modes wrap and bare with bare being more performant
  • However bare mode can't be used on graphql sources, any transform on graphql sources will always use wrap mode no matter the configuration

Related issues: #1891 #1928

@santino
Copy link
Contributor

santino commented Apr 9, 2021

Some quick comments.
The only transforms that do not support wrap mode are: cache, extend and snapshot
I tested and they do not work with GraphQL sources.

Actually extend is not meant to be used with GraphQL sources, it has been introduced purely to offer a way to do schema extensions to bare schemas.
For schema extensions that work across all sources the approach is not to use the transform, but to use additionalTypeDefs and additionalResolvers , as documented in the relevant page.

So the only transforms to introduce wrap to are cache and snapshot, and it would be trivial to implement this mode, since they are actually agnostic of data source, since they operate on the response and do not care about the source schema at all.

On another note, "bare" mode is not available in many more transforms: encapsulate, federation, mock, naming-conventions, and prefix.

As about this point

any transform on graphql sources will always use wrap mode no matter the configuration

It does sound nice, but probably difficult to implement.
Transforms are meant to be source agnostic, since they mostly work on generated graphql schemas, and so they're not aware of the source. This means the transform doesn't know which source it is applied to.

Might need some thinking.

@theguild-bot theguild-bot mentioned this issue Aug 11, 2022
@theguild-bot theguild-bot mentioned this issue Sep 28, 2023
This was referenced Apr 30, 2024
This was referenced May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants