Usage of legacy `.graphql_definition` objects on SchemaPlugin#authenticate_option

[janraasch]

What is the problem the enhancement will solve?

After upgrading to graphql_devise 0.18.0 I am getting the following deprecation message from graphql 1.13.10:

Legacy `.graphql_definition` objects are deprecated and will be removed in GraphQL-Ruby 2.0. Remove `.graphql_definition` to use a class-based definition instead.

Called on #<Types::BaseField Query.myquery(...): [Mytype!]!> from:
   /Users/jan/.rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/graphql_devise-0.18.0/lib/graphql_devise/schema_plugin.rb:116:in `authenticate_option'
  /Users/jan/.rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/graphql_devise-0.18.0/lib/graphql_devise/schema_plugin.rb:30:in `trace'

Context

I set up our Schema like so

class MyServerSchema < GraphQL::Schema
  use GraphqlDevise::SchemaPlugin.new(
    query: Types::QueryType,
    mutation: Types::MutationType,
    public_introspection: true,
    resource_loaders: [
      GraphqlDevise::ResourceLoader.new(User,
                                        only: %i[login logout register update_password_with_token
                                                 send_password_reset_with_token])
    ]
  )

  mutation(Types::MutationType)
  query(Types::QueryType)

Describe the solution you have in mind

Not sure 😇. Judging from the deprecation message, we could simply remove the call to .graphql_definition 🤓...

Describe alternatives you've considered

To have less noise on our test logs, I might choose to downgrade again...

Additional context

• Here's a link to the respective call on this repo: https://github.com/graphql-devise/graphql_devise/blob/master/lib/graphql_devise/schema_plugin.rb#L82
• GQL 1.13 support pr: https://github.com/graphql-devise/graphql_devise/pull/211/files

Please let me know, if you need more details.

[mcelicalderon]

Thank you for the report, @janraasch! We didn't change that in 0.18, so perhaps you also upgraded the graphql gem that has now deprecated that method? Anyway, we should fix that in preparation for the release of GraphQL 2.0

[janraasch]

Hi @mcelicalderon, thank you for replying so quickly 🙏.

Thank you for the report, @janraasch! We didn't change that in 0.18, so perhaps you also upgraded the graphql gem that has now deprecated that method?

Yes, I did indeed upgrade graphql as well. Being able to upgrade graphql to v1.13 was the main reason for upgrading graphql_devise to 0.18.0 for me, see #205 (comment).

I'd love to see a fix for this deprecation message even in a 0.18.1-patch release. What do you think ☺️?

Anyway, we should fix that in preparation for the release of GraphQL 2.0

👍

[mcelicalderon]

Not using that method should be simple enough, so if we get the capacity to work on that, we can definitively release a patch version with just that. But who knows, v1 is near, so that might happen first? 🙌 🎉

Anyway, leaving this one open until the deprecation is fixed.

[janraasch]

Not using that method should be simple enough, so if we get the capacity to work on that, we can definitively release a patch version with just that.

👌. If you could gimme some pointers as to what needs to be done, I might be able to give it a try.

I just took a few minutes:

• removed the call,
• ran the specs, but

I am not familiar enough with the inner workings of graphql to have the errors point me into a direction for a fix 😇.

But who knows, v1 is near, so that might happen first? 🙌 🎉

Very nice 🐎 😁.

---

For later reference: Here's a commit on graphql rmosolgo/graphql-ruby@61787ee removing all .define-related code.

[mcelicalderon]

removed the call

That has to be replaced with something else of course 😄, but if you want to dig a little deeper, I think it all comes down to this

graphql_devise/lib/graphql_devise/schema_plugin.rb

Line 79 in 502c555

auth_required = if trace_data[:context]

I can't recall right now, but I think the conditional there was precisely to work with 2 different versions of the GraphQL gem. So perhaps now we need to check the gem's version there before using the new mechanism

[mcelicalderon]

I looked a bit onto this and found that supporting GraphQL 2.0 will be a bit harder than expected. Apparently we are not going to be able to do something like:

field :private, String, null: true, authenticate: true

We'll no longer be able to .accept_defintion of authenticate at the field level.

There's a related issue where the author of the GraphQL gem gives some alternatives in exAspArk/graphql-guard#53

So, as this is going to be a breaking change of the existing interface, we won't be able to ship a fix for this before v1.0.0

@janraasch

[janraasch]

Hi @mcelicalderon, thank you for looking into this.

we won't be able to ship a fix for this before v1.0.0

idea What do you think about then hiding the deprecation message via silence_deprecation_warning: true (see rmosolgo/graphql-ruby@3d1c24a) in a 0.18.x-release?

reasoning As a user of this library I cannot do anything about this deprecation message => It only? adds noise to my logs.

Let me know what you think 🙏

[mcelicalderon]

That makes sense, @janraasch! I pushed an MR with the change but I think I need to do a version check so the build doesn't break, I'll take another look as soon as possible. I'll release a new version with the change. Logs are important 🙌

You are welcome to propose a change if you can get to it before I do!

• New branch based on release
• Import GQL 1.13 support changes into release branch (test matrix and appraisals so we can make sure it works)
• Check which GQL version introduces the silence_deprecation_warning argument and call with the argument only if the version is greater or equal than that.

[mcelicalderon]

Closing this one as the deprecation message was silenced. Thank you, @janraasch!