graphql-schema_comparator icon indicating copy to clipboard operation
graphql-schema_comparator copied to clipboard

Descriptions with new lines are listed as SafeChange

Open 23tux opened this issue 6 years ago • 3 comments

When using long descriptions, e.g.

value "width", "Resizes the image to given width (in pixels) and keeps proportions. Resizing the image bigger than the original is not possible.", value: :width

and dumping the schema, it looks like this

enum ImageOperatorFilter {
  """
  Resizes the image to given width (in pixels) and keeps proportions. Resizing
  the image bigger than the original is not possible.
  """
  width
}

because of the formatting in GraphQL::Language::BlockString, which adds a new line after a certain amount of characters.

Unfortunately, graphql-schema_comparator lists this new line as a change

Safe change:: Description for enum value `ImageOperatorFilter.width` changed from `Resizes the image to given width (in pixels) and keeps proportions. Resizing
the image bigger than the original is not possible.` to `Resizes the image to given width (in pixels) and keeps proportions.`

So now, you can't use graphql-schema_comparator any more because it always detects a change, even if there isn't one.

Would it be possible to ignore such new lines in descriptions? Or would it be better to not include newlines in the schema, and fix this on graphql-ruby gem side?

23tux avatar Oct 01 '19 05:10 23tux

Thanks for bringing this up!

Would it be possible to ignore such new lines in descriptions? Or would it be better to not include newlines in the schema, and fix this on graphql-ruby gem side?

Interesting. I feel like any change in the description string is indeed a change 🤔

you can't use graphql-schema_comparator any more because it always detects a change, even if there isn't one.

I'm trying to understand why you can't use it anymore. The first time you change to a description with new line, I would expect there would be description changes, but once you merge, I feel like there would be no more changes the next time? Am I getting this right?

xuorig avatar Oct 02 '19 17:10 xuorig

Consider this test for RSpec

  it "looks if graphql schema has changed" do
    old_schema = File.read(Rails.root.join("schema.graphql"))
    new_schema = MySchema.to_graphql
    result = GraphQL::SchemaComparator.compare(old_schema, new_schema)
    expect(result).to be_identical
  end

When we have a change in the GraphQL schema, we dump it with rails graphql:schema:idl. When we have a long description, it get's dumped with new lines into the schema.graphql file. When you re-read it in the spec to check if there are changes, the MySchema.to_graphql has NO new lines, but the schema read from schema.graphql has the new lines.

So it detects a change, even if there is none. Do you know what I mean?

23tux avatar Oct 02 '19 18:10 23tux

Ran into this issue myself just now: a couple of our fields have long enough descriptions that they are word wrapped in the schema.graphql file, but the GraphQL Schema object does not include the wrapping.

We had to work around this issue by explicitly ignoring specific "changes" that would incorrectly get flagged due to the descriptions being text-wrapped in the schema dump, which is brittle thing that it would be better not to have to maintain:

        false_alarm = true
        compare_result.changes.each do |change|
          if change.is_a?(GraphQL::SchemaComparator::Changes::InputFieldDescriptionChanged) &&
              change.old_field.description.tr("\n", " ") == change.new_field.description
            # This is a false alarm non-change due to text formatting done when dumping
            # to the schema.graphql file for long descriptions, so we skip it
            next
          end
          if change.is_a?(GraphQL::SchemaComparator::Changes::FieldArgumentDescriptionChanged) &&
              change.old_argument.description.tr("\n", " ") == change.new_argument.description
            # This is a false alarm non-change due to text formatting done when dumping
            # to the schema.graphql file for long descriptions, so we skip it
            next
          end

          puts "    #{change.message}"
          false_alarm = false
        end

CodingAnarchy avatar Aug 12 '22 01:08 CodingAnarchy