Skip to content

add support for Apollo @rest directive #20

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KitoC
Copy link

@KitoC KitoC commented Oct 15, 2019

Issue
Apollo rest-link uses the @rest directive to to determine if it is a rest query/mutation with a range of config passed into the args of the directive ( e.g. @rest(path: "/foo")). Currently if you want to use both __directive and __args for the same key, json-to-graphql-query will generate the following directive and arg shape foo @rest(directive: args) (real: args). Apollo requires you to construct it like this - foo (real: args) @rest(directive: args).

Additionally, as far as I am aware, there doesn't seem to be any reason for stripping the quotes " from the args passed into the directive. Currently if you want to have a string in the directive args you cannot.

Solution

  1. Add a conditional to check for an @rest directive and place the args in front if @rest is present.
  2. remove the quote replacer

This is my first time contributing to open source projects so if there is anything I missed please let me know 😁 👍 .

@KitoC KitoC changed the title - added @rest directive arg placement and removed quote replacer on d… add support for Apollo @rest directive Oct 15, 2019
@dupski
Copy link
Collaborator

dupski commented Jan 27, 2020

Thanks @KitoC - apologies for not having looked at this, I'm not doing much JS at the moment! I will try to take a look in the next week or so :)

@dupski
Copy link
Collaborator

dupski commented Feb 3, 2020

Hi @KitoC - just had a look at this (again, apologies for how long this has taken, particularly as it was your first contribution!!). I think we need something a bit more generic than tie-ing it to a specific directive name.

I wonder if we should just add an extra key in the directive args object, e.g. __position which could take "beforeArgs" or "afterArgs" as a value to control the position of the directive.

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