graphql-client: `undefined method []'` when parsing query in graphql-ruby 2.1.0

Parse fails when upgrading to graphql 2.1.0. It is most likely a change in the Visitor hooks API that is Breaking changes.

https://github.com/rmosolgo/graphql-ruby/blob/master/CHANGELOG.md#210-30-aug-2023

Visitor: legacy-style proc-based visitors are no longer supported #4577 #4583

https://github.com/rmosolgo/graphql-ruby/pull/4577

I’ve written a script to demonstrate the error across two versions of the gem

require "bundler/inline"

gemfile do
  source 'https://rubygems.org'

  if ENV["BEFORE"]
    puts "using graphql 2.0.26"
    gem "graphql-client", "0.18.0"
    gem "graphql", "2.0.26"
  elsif ENV["AFTER"]
    puts "using graphql 2.1.0"
    gem "graphql-client", "0.18.0"
    gem "graphql", "2.1.0"
  else
    raise "Pass BEFORE=1 or AFTER=1"
  end
end

class Query <  GraphQL::Schema::Object
  field :hoge, String, null: false
  def hoge
    "hoge"
  end
end

class Schema < GraphQL::Schema
  query Query
end

client = GraphQL::Client.new(schema: Schema, execute: Schema)

QUERY = client.parse(<<-'GRAPHQL')
  query {
    hoge
  }
GRAPHQL

p result.data.hoge

Here is the error trace that occurs.

/Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-client-0.18.0/lib/graphql/client/query_typename.rb:28:in `add_typename': undefined method `[]' for nil:NilClass (NoMethodError)

            type = @types[node]
                         ^^^^^^
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-client-0.18.0/lib/graphql/client/query_typename.rb:48:in `on_operation_definition'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:95:in `on_operation_definition_with_modifications'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:121:in `public_send'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:121:in `block in on_document_children'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:119:in `each'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:119:in `on_document_children'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:84:in `on_document'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:95:in `on_document_with_modifications'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:53:in `public_send'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-2.1.0/lib/graphql/language/visitor.rb:53:in `visit'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-client-0.18.0/lib/graphql/client/query_typename.rb:62:in `insert_typename_fields'
        from /Users/nat/.anyenv/envs/rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/graphql-client-0.18.0/lib/graphql/client.rb:189:in `parse'
        from main.rb:32:in `<main>'

About this issue

Original URL
State: open
Created 10 months ago
Reactions: 18
Comments: 16 (3 by maintainers)

Commits related to this issue

Try installing a working version of graphql gem Because of https://github.com/github/graphql-client/issues/310 — committed to pupilfirst/ai-review-action by harigopal 10 months ago
Lock the graphql version for now More details in https://github.com/github/graphql-client/issues/310 — committed to yuki24/artemis by yuki24 9 months ago
Update dependencies This pins the version for them `graphql` gem, as version 2.1.0 has a bug that breaks query parsing. We can track progress on this in issue [#311](https://github.com/github/graphql... — committed to MITLibraries/timdex-ui by jazairi 9 months ago
Update dependencies This pins the version for them `graphql` gem, as version 2.1.0 has a bug that breaks query parsing. We can track progress on this in issue [#311](https://github.com/github/graphql... — committed to MITLibraries/timdex-ui by jazairi 9 months ago
Downgrade graphql https://github.com/github/graphql-client/issues/310 — committed to hovancik/stretchly_server by hovancik 7 months ago

Most upvoted comments

CI is a bit … dusty … but I got several key tests to run on this branch: https://github.com/github/graphql-client/pull/314

Could someone give that a try and let me know how it goes for them? You can bundle it like this:

gem "graphql-client", github: "rmosolgo/graphql-client", ref: "27ef61f"

I’ve migrated everything I could find, but I’m just working from the test files that I could get to run on GraphQL-Ruby 2.1 😅

+12

rmosolgo on Sep 19, 2023

@zavan same problem with graphlient

I had set a limit to the graphql version in the Gemfile:

gem "graphql",     '>= 2.0.27',  '< 2.1'

rellampec on Sep 6, 2023

CI is a bit … dusty … but I got several key tests to run on this branch: #314

Could someone give that a try and let me know how it goes for them? You can bundle it like this:
gem "graphql-client", github: "rmosolgo/graphql-client", ref: "27ef61f"
I’ve migrated everything I could find, but I’m just working from the test files that I could get to run on GraphQL-Ruby 2.1 😅

Haven’t tested further than locally and in CI but this looks to be working well so far @rmosolgo, thanks!

mitchpascoe on Sep 20, 2023

Yeah, we removed the deprecated visitor [node_class] << ... API in GraphQL-Ruby 2.1, including the internal TypeStack visitor.

I started migrating those visitors in graphql-client just now and found that GraphQL-Client has other incompatibilities in its test suite:

MiniTest -> Minitest
It still uses .define-based GraphQL definitions which have been removed since GraphQL-Ruby
use GraphQL::Execution::Interpreter needs to be removed; it’s default now and can’t be added that way

Anyhow, here’s an example migration:

diff --git a/lib/graphql/client.rb b/lib/graphql/client.rb
index 83bad2d..9c3bf68 100644
--- a/lib/graphql/client.rb
+++ b/lib/graphql/client.rb
@@ -425,12 +425,26 @@ module GraphQL
       end.to_h
     end

+    class GatherNamesVisitor < GraphQL::Language::Visitor
+      def initialize(node)
+        @names = []
+        super
+      end
+
+      attr_reader :names
+
+      def on_fragment_spread(node, parent)
+        @names << node.name
+        super
+      end
+    end
+
     def find_definition_dependencies(node)
-      names = []
-      visitor = Language::Visitor.new(node)
-      visitor[Language::Nodes::FragmentSpread] << -> (node, parent) { names << node.
name }
+      visitor = GatherNamesVisitor.new(node)
       visitor.visit
-      names.uniq
+      names = visitor.names
+      names.uniq!
+      names
     end

There are several others that need the same treatment:

Migrate the Language::Visitor instance to a custom subclass
Move the [node_class] << -> ... hooks into on_{node_class} methods
Migrate the visitor’s state to use instance variables on the custom subclass instead of local variables in the proc

rmosolgo on Aug 31, 2023

I’m assuming https://github.com/ashkan18/graphlient has the same problem, since it’s based on this gem?

zavan on Sep 6, 2023