search

graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.04k stars 2.02k forks source link

Require non-empty directive locations #4100

Closed jbellenger closed 3 months ago

jbellenger commented 4 months ago

A proposed spec edit clarifies a requirement that was always true but was slightly buried: directive definitions must include 1 or more locations.

This PR adds explicit validation to graphql-js around non-empty directive locations. Similar work was landed in graphql-java.

I'll add that I haven't contributed to graphql-js before and am not familiar with typescript or the mores of graphql-js. I've tried to follow existing patterns but would appreciate any feedback offered.

netlify[bot] commented 4 months ago

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
Latest commit 6160b4dd257fc2cd06e244e77950ad835e6f6995
Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/665cc53c2ab985000819cde6
Deploy Preview https://deploy-preview-4100--compassionate-pike-271cb3.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions[bot] commented 3 months ago

Hi @jbellenger, I'm @github-actions bot happy to help you with this PR 👋

Supported commands Please post this commands in separate comments and only one per comment: * `@github-actions run-benchmark` - Run benchmark comparing base and merge commits for this PR * `@github-actions publish-pr-on-npm` - Build package from this PR and publish it on NPM