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.

Sign up for GitHub

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

Jump to bottom

Directives must appear exactly once on a field #156

Closed
Fi1osof opened this issue Aug 29, 2018 · 3 comments · Fixed by #185 · May be fixed by saurabharch/meteor-apollo-starter-kit#2 or notadd/graphql#1
Closed

Directives must appear exactly once on a field #156

Fi1osof opened this issue Aug 29, 2018 · 3 comments · Fixed by #185 · May be fixed by saurabharch/meteor-apollo-starter-kit#2 or notadd/graphql#1
Labels
🐞bug Something isn't working 🎓good for beginners ⛑️help wanted Extra attention is needed

Comments

@Fi1osof
Copy link
Contributor

Fi1osof commented Aug 29, 2018

Mayby bug.

Try to merge twice

type User {
  id: ID! @unique
  fullname: String
}

type User {
  id: ID! @unique
  email: String
}

Got:

schema type User {
  id: ID! **@unique @unique**
  fullname: String
}

Error: The field id specifies a directive more than once. Directives must appear exactly once on a field.
@RichardLitt RichardLitt added the 🐞bug Something isn't working label Aug 29, 2018
@cfnelson cfnelson added the ⛑️help wanted Extra attention is needed label Sep 11, 2018
@cfnelson
Copy link
Contributor

Thanks for reporting this behaviour.

The behaviour was introduced in PR #144 and it looks like we don't have a test for this.

The issue I believe will be due to this line https://github.com/okgrow/merge-graphql-schemas/pull/144/files#diff-3b4b88545526e76374a7547d901a272eR75 potentially the solution could be as simple as ensuring the directives are unique before concating them together.

This would likely be a good PR for anyone wishing to contribute to the project for the first time.

@ttapar
Copy link

ttapar commented Oct 2, 2018

Do you mind if I check this out?

@RichardLitt
Copy link
Contributor

@ttapar Go ahead! Let us know if you need help.

@cfnelson cfnelson mentioned this issue Oct 26, 2018
Closed
cfnelson added a commit that referenced this issue Nov 12, 2018
@cfnelson
Merge pull request #183 from Fi1osof/patch-156
37bece6 
Fix #156
@cfnelson cfnelson mentioned this issue Nov 14, 2018
Merged
@snyk-bot snyk-bot mentioned this issue Feb 23, 2020
Open
@snyk-bot snyk-bot mentioned this issue Mar 13, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐞bug Something isn't working 🎓good for beginners ⛑️help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
4 participants
@RichardLitt @Fi1osof @cfnelson @ttapar