Detect new interface implementation for new types

[jturkel]

This PR fixes a bug where dangerous new interface implementations were not detected for new types. While I was in here, I changed the kitchen sink test to compare strings rather than arrays of strings so test failures would print a nice diff.

[swalkinshaw]

This is actually a subset of https://github.com/xuorig/graphql-schema_comparator/issues/33. Ideally we'd capture every change for a new type?

[jturkel]

🤔 You're right there's definitely overlap between this issue and #33 but do you have any insight into the use cases where reporting nested changes would be generally useful?

My main concern is applying a recursive strategy more broadly could result in a large number of changes being reported making it harder for developers to understand the root cause of lots of warnings. For example when a type is removed, the recursive strategy would report a change for the type's removal, a change for each of its fields being removed, and a change for each its fields' arguments being removed. Perhaps we could return a tree of changes (e.g. the type removed change node would have child change nodes for each of the removed type's fields) and leave it up to clients how they want to process that tree (e.g. whether or not to stop the recursion when they hit the first breaking change node)? I'm just not sure it buys a client much over walking the graphql-ruby types though.

On a somewhat related note, I'm beginning to wonder if removing a type is actually a breaking change. It seems like the real breakage is from removing fields on other types that could break selection sets or removing an interface implementation that could break fragments. Admittedly this is probably biased by my use case of a GitHub Action that generates annotations with links to details on impacted clients for any breaking/dangerous changes though.

[swalkinshaw]

Our use case is a GitHub bot which ran linting on PRs. We defined hooks based on each type of change (ie: on_field_added) which a lint rule would implement (similar to how Rubocop works). Without this nested/recursive behaviour, we'd have to explicitly implement both on_field_added and a more verbose on_type_added to handle new types with fields. If the gem always generated Change::FieldAdded for fields on new types (for example), this would happen automatically.

Your proposal is basically what we did internally in our bot app so yeah clients should be able to decide what they care about or not.

On a somewhat related note, I'm beginning to wonder if removing a type is actually a breaking change. It seems like the real breakage is from removing fields on other types that could break selection sets or removing an interface implementation that could break fragments.

This sort of mirrors how deprecations work since you don't actually deprecate a type, just the fields that return them. I guess there's some nuance since removing a type can mean:

• a union's possible types change
• types an interface implement change
• fragments (as you mentioned)

So imo it's still a breaking change to remove a type.