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

Add 5.2.4 Operation Type Exists #1098

Closed
wants to merge 7 commits into from
Closed

Conversation

Shane32
Copy link

@Shane32 Shane32 commented May 27, 2024

Closes #1097

Copy link

netlify bot commented May 27, 2024

Deploy Preview for graphql-spec-draft ready!

Name Link
🔨 Latest commit b9f1f33
🔍 Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/67007a357700f600088a42dc
😎 Deploy Preview https://deploy-preview-1098--graphql-spec-draft.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.

@benjie benjie added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label Jun 4, 2024
benjie
benjie previously requested changes Jun 4, 2024
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find!

Co-authored-by: Benjie <benjie@jemjie.com>
@Shane32 Shane32 changed the title Add 5.2.4 Operation Type Configuration Add 5.2.4 Operation Type Exists Jun 4, 2024
@benjie benjie dismissed their stale review June 5, 2024 10:16

All points were addressed

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, though some slight editorial on the wording to match the style of the spec as a whole might be warranted.

Copy link
Member

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great validation rule 🙌

@benjie
Copy link
Member

benjie commented Sep 19, 2024

The lack of this validation rule can be demonstrated in GraphQL.js with this:

import { GraphQLSchema, GraphQLInt, graphql, GraphQLObjectType } from 'graphql';

const Query = new GraphQLObjectType({
  name: 'Query',
  fields: {
    a: {
      type: GraphQLInt,
      resolve() {
        return 42;
      },
    },
  },
});

const schema = new GraphQLSchema({
  query: Query,
});

const result = await graphql({ schema, source: `mutation { __typename }` });
console.dir(result);

which produces:

{
  data: null,
  errors: [
    GraphQLError: Schema is not configured to execute mutation operation.
        at executeOperation [...]
      path: undefined,
      locations: [Array],
      extensions: [Object: null prototype] {}
    }
  ]
}

We should really mark this as an invalid request, and thus it should not have a data key in the response.

@fotoetienne fotoetienne added 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) and removed 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) labels Oct 3, 2024
@yaacovCR
Copy link
Contributor

This is a refresh of #955 which has a pending PR at the implementation graphql/graphql-js#3601

@benjie
Copy link
Member

benjie commented Oct 17, 2024

Good spot @yaacovCR; it seems both @Shane32 and I had commented on that previous PR!

I have merged this PR into that PR and applied my own editorial edits on top of both:

@benjie benjie closed this Oct 17, 2024
yaacovCR added a commit to graphql/graphql-js that referenced this pull request Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No validation rule exists to assert mutation/subscription operations exist in the schema
5 participants