Skip to content

Add wrap mode to resolvers-composition transform #1928

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

Merged
merged 2 commits into from
Apr 12, 2021

Conversation

santino
Copy link
Contributor

@santino santino commented Apr 9, 2021

As per the title, this PR introduces "wrap" mode to the resolvers-composition transform.
This is in order to make this transform work on GraphQL sources.
The PR is related to #1891.

I tested the changes and updated and extended the documentation.

I introduced a new syntax for the transform but also kept backward compatibility with the previous syntax, this is in line with other transforms that support both "bare" and "wrap" modes.
The new syntax is as follow, note the new "mode" property, and the array that is now wrapped under "compositions" property:

transforms:
  - resolversComposition:
      mode: bare | wrap
      compositions:
        - resolver: 'Query.me'
          composer: is-auth#isAuth
        - resolver: 'Mutation.*'
          composer: is-admin#isAdmin

Important thing to note:

This transform didn't support "wrap" mode prior to this PR. However, the convention established for transforms is to work in "wrap" mode by default.
This means that now that the transform supports "wrap", I made this the default mode; hence if "mode" property is not specified, then "wrap" will be applied by default.

This is consistent with other transforms that support both modes, however, it might have undesired effects for users that are currently using this transform.
Potentially we could make default mode for this transform to be "bare", to avoid a change in behavior for existing users; however, that would become inconsistent with other transforms.

I believe we need to stick to the decision made to operate in "wrap" by default.

Once agreed, I will add the changeset and specify the potentially breaking behavior (not breaking changes in terms of API though).

@changeset-bot
Copy link

changeset-bot bot commented Apr 9, 2021

⚠️ No Changeset found

Latest commit: 20b4c88

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ardatan
Copy link
Owner

ardatan commented Apr 12, 2021

Let's merge this first and discuss more on the issue then :) Thanks @santino !

@ardatan ardatan merged commit 9dd6152 into ardatan:master Apr 12, 2021
ardatan pushed a commit that referenced this pull request Apr 12, 2021
* Implement "wrap" mode on resolvers composition transform

so this works on GraphQL sources as well

* Update documentation
@santino santino deleted the resolvers-composition-wrap branch April 22, 2021 09:33
klippx pushed a commit to klippx/graphql-mesh that referenced this pull request Oct 9, 2024
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

2 participants