Skip to content

fix: allow repeatable directives on Graphql document#418

Open
fredzqm wants to merge 1 commit intovektah:masterfrom
fredzqm:master
Open

fix: allow repeatable directives on Graphql document#418
fredzqm wants to merge 1 commit intovektah:masterfrom
fredzqm:master

Conversation

@fredzqm
Copy link
Contributor

@fredzqm fredzqm commented Mar 10, 2026

Before

Previously, the UniqueDirectivesPerLocation rule was checking dir.Name != "repeatable", which was incorrect.
It only allowed directive @repeatable to be repeated.

After

It should look for dir.Definition.IsRepeatable, which allows directive @check repeatable on FIELD to be specified many times in the same place.

Added test cases for:

  • A custom repeatable directive used multiple times (expected no error).
  • A custom non-repeatable directive used multiple times (expected error).

Describe your PR and link to any relevant issues.

I have:

  • Added tests covering the bug / feature
  • Updated any relevant documentation

Fixes a panic when validating directives that are not defined in the schema.
Previously, the UniqueDirectivesPerLocation rule would panic if a directive
did not have a definition, as it tried to access .

This change adds a nil check for  before accessing .
A directive is considered non-repeatable if its definition is not found or
if its definition explicitly states it is not repeatable.

Added test cases for:
- A custom repeatable directive used multiple times (expected no error).
- A custom non-repeatable directive used multiple times (expected error).
@coveralls
Copy link

Coverage Status

coverage: 87.218%. remained the same
when pulling 36e00c9 on fredzqm:master
into d828d64 on vektah:master.

@fredzqm
Copy link
Contributor Author

fredzqm commented Mar 12, 2026

@StevenACoffman [not in a rush]

I think I may have caught a bug in the current parser. Let me know what you think~

Thank you!

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