graphql / graphql-js

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

is OPTIONAL_ARG_ADDED a breaking change ? #3829

Closed spydercavern closed 1 month ago

spydercavern commented 1 year ago

OPTIONAL_ARG_ADDED a breaking change ?

Can any one help regarding the rational for OPTIONAL_ARG_ADDED as a breaking change ?

it('should detect if an optional field argument was added', () => { const oldSchema = buildSchema( type Type1 { field1(arg1: String): String } );

const newSchema = buildSchema(`
  type Type1 {
    field1(arg1: String, arg2: String): String
  }
`);

expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([
  {
    type: DangerousChangeType.OPTIONAL_ARG_ADDED,
    description: 'An optional arg arg2 on Type1.field1 was added.',
  },
]);

https://www.apollographql.com/docs/graphos/delivery/schema-checks/#schema-additions implementation list this change as non breaking change as well, which i agree too as well.

yaacovCR commented 1 year ago

It is listed as a dangerous change, rather than breaking, right?

I am not sure why it is called "dangerous" as opposed to simply a change, but that is the only other category. :)

JoviDeCroock commented 1 month ago

It shouldn't be marked as a breaking change, I don't think it should even be marked as a dangerous change. I guess the dangerous change could be justified if we say that we split up functionality, however that's on the resolver/logic side.

yaacovCR commented 1 month ago

Backstory here: https://github.com/graphql/graphql-js/pull/1096 The idea was that you might have to update client code.