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

Descriptions with new lines are listed as SafeChange #27

Open
23tux opened this issue Oct 1, 2019 · 3 comments
Open

Descriptions with new lines are listed as SafeChange #27

23tux opened this issue Oct 1, 2019 · 3 comments

Comments

@23tux
Copy link

23tux commented Oct 1, 2019

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?

@xuorig
Copy link
Owner

xuorig commented Oct 2, 2019

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?

@23tux
Copy link
Author

23tux commented Oct 2, 2019

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?

@CodingAnarchy
Copy link

CodingAnarchy commented Aug 12, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants