Adding an interface to a type is now a dangerous change.

[excitement-engineer]

Following the discussion in issue #968, adding an interface to a type is now considered a dangerous change.

[leebyron]

I'm not sure that this would also qualify as a "dangerous" change in the same way as others - could you help to clarify?

[excitement-engineer]

I think that it could be a dangerous change if branching on the __typename of the implementing type is not implemented defensively.

See my launchpad example. In this example we have 2 implementing types for the interface User: Account and Organization. We then perform the query illustrated in the launchpad for retrieving a list of users that we want to show in the UI.

Say that a client uses the __typename to determine whether a User is an Account or Organization and show a different UI accordingly. If the User interface is added to a new type, say a Bot, then the __typename has 3 possible values instead of 2 which may break clients that do not implement branching on __typename defensively.

Is my reasoning correct here @leebyron? Curious to hear what you think!

[mjmahone]

@leebyron I believe this is a known "dangerous change" (not breaking, but dangerous) to us as well. I believe @sam-swarr has more context. There's also the "dangerous change" of adding a field to an input object type.

The above is with the assumption that "dangerous change" means "it could affect your client if you built things poorly (i.e. didn't provide a default in a switch statement), but won't affect clients built with the concept of GraphQL breaking changes in mind".

Basically: if something breaks due to a "dangerous change" there's some underlying, root issue on your client you should dig in and fix so future changes don't cause that problem. But not coding defensively can lead to these issues. Whereas if you make a "breaking change" it's expected that clients won't be able to handle it correctly.

[sam-swarr]

I agree with @mjmahone's take on the difference between dangerous/breaking changes. Given that adding a new interface to an object type can break client code (if it's not designed defensively to handle future values), I think this is a valid addition to findDangerousChanges. I see the same parallels with adding a new value to an enum, which is already part of findDangerousChanges.

[mjmahone]

Thanks for helping identify and push forward this change, @excitement-engineer!

[schafle]

@excitement-engineer Your comment here #992 (comment) and the code does not appear to be in sync. Following test case tries to reproduce the scenario you mention in the comment but the test fails:

  it('new type is added to existing interface', () => {
    const oldSchema = buildSchema(`
      interface User

      type Account implements User
      type Organization implements User
    `);

    const newSchema = buildSchema(`
      interface User

      type Account implements User
      type Organization implements User
      type Bot implements User
    `);

    expect(findDangerousChanges(oldSchema, newSchema)).to.deep.equal([
      {
        type: DangerousChangeType.IMPLEMENTED_INTERFACE_ADDED,
        description: 'New type is added to existing interface',
      },
    ]);
  });