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

Directive "@deprecated" for arguments in SDL is ignored #317

Closed
vladar opened this issue Feb 11, 2021 · 3 comments
Closed

Directive "@deprecated" for arguments in SDL is ignored #317

vladar opened this issue Feb 11, 2021 · 3 comments
Labels

Comments

@vladar
Copy link
Contributor

vladar commented Feb 11, 2021

Looks like the @deprecated directive for arguments in SDL is ignored. Check out the example:

const typeComposer = typeMapper.convertSDLTypeDefinition(`
  type Foo {
    foo(
      arg: String @deprecated(reason: "Tired")
    ): String
  }
`);

Now if I try building this type it renders without the @deprecated directive:

import { printType } from "graphql";

printType(typeComposer.getType());

// prints this:
//   type Foo {
//     foo(arg: String): String
//   }

I wanted to fix it and open a PR but looks like this is intentional:

if (name === GraphQLDeprecatedDirective.name) {
// @deprecated directive should be parsed via getDeprecationReason() method
// It's due to fact that deprecated is stored as separate type instance's field.
return;
}

So not sure what is the best course of action here. Any thoughts?

@nodkz
Copy link
Member

nodkz commented Feb 11, 2021

I've spent several hours trying to figure out this problem without lack.
If we remove this logic the the test 254 fails with the following error (duplication of deprecated directive):
Screen Shot 2021-02-12 at 02 59 29

Even more, there is another problem with directives for args. When I remove 1120-1124 lines then printType method does not add directive properly.

@vladar may I ask you to open a new PR with your fix? Please ignore the 254 test suite I'll fix it myself and add the following test suite:

/* @flow */

import { printType } from 'graphql';
import { schemaComposer, dedent } from '../..';

describe('github issue #317: Directive @deprecated for arguments in SDL is ignored ', () => {
  it('should build schema successfully', async () => {
    const typeComposer = (schemaComposer.typeMapper.convertSDLTypeDefinition(`
      type Foo {
        foo(
          arg: String @deprecated(reason: "Tired")
        ): String
      }
    `): any);

    const type = typeComposer.getType();

    expect(printType(type)).toEqual(dedent`
      type Foo {
        foo(
          arg: String @deprecated(reason: "Tired")
        ): String
      }
    `);
  });
});

@vladar
Copy link
Contributor Author

vladar commented Feb 14, 2021

I switched to something else at the moment. But will try to come up with a PR next week.

nodkz added a commit that referenced this issue May 30, 2021
@nodkz nodkz closed this as completed in 1ea2d55 May 30, 2021
@github-actions
Copy link

🎉 This issue has been resolved in version 9.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants