Graphql-ruby: Release 1.10.0

Created on 11 Feb 2019  路  10Comments  路  Source: rmosolgo/graphql-ruby

Here are some goals I have for 1.10.0. Please feel free to contribute to the conversation by leaving a comment.

  • Redo pagination, see #1359

    • [x] Implement new system #2143

    • [x] Turn it on by default Add it to generator

    • [x] Improve docs (conceptual & how-to)

  • [x] Remove the schema build phase, instead, construct the type caches _during_ configuration methods.

    • In the works at #2363

    • The goal is to fix the thread-safety issues that result from a lazily-constructed schema

    • Also, it will move GraphQL::Schema _away_ from an internal dependence on .define

  • Consider a built-in batching implementation

    • Include features from #1981

    • In the works at #2483

    • I'm going to cut this because it's time to get these other changes released!

  • [x] #2178 Update Schema.from_definition to build a class-based schema

    • Mainly to remove the internal dependence on .define.

  • [x] Update some specific docs, which were added a long time ago (this can happen during the 1.10 cycle)

    • [x] #2177 Testing: add a whole section with tips and examples

    • [x] https://github.com/rmosolgo/graphql-ruby/pull/2179 Dynamic definition: just remove it?

    • Backtrace annotations (todo, port this to interpreter?)

    • [x] #2174 Instrumentation: remove it

    • [x] #2175 Remove the blue notice about class-based API being new

  • [x] Mark legacy .define-based API docs as @api deprecated
  • [x] update the schema generator to default to the interpreter etc.
  • [x] Figure out when interpreter is required? (Is it required for printing class-based schemas?)
  • [x] Run Ruby 2.7 with no warnings

Most helpful comment

Regarding pagination, the goals you have laid out in #1359 to support a clear and more explicit mapping for cursor based pagination is :+1:. It took us a long while to decode and extend the existing relay pagination for our own purposes. The method names, purposes, and the integration with GraphQL were not very clear.

:+1: to batching implementation. In particular, if (:crossed_fingers:) the concurrency abstraction is included in the next release, having a thread-safe batch implementation will be increasingly important. One more thing I would add here, existing implementations of loaders (like graphql-batch) effectively require an entirely new class to be created for each query or network call. This can become very cumbersome, so it would be great to provide a more lightweight way to have concurrent and batch loading lazy objects.

Regarding testing, we have found it difficult to unit test some things (like authorization) for somewhat nested objects because we need to write and execute full queries (instead of simply instantiating the graphql type with a context). Some kinds of helpers or easy ways to instantiate and unit test isolated graphql types would be great. Maybe this is already possible and we just need some examples :grinning:.

All 10 comments

Regarding pagination, the goals you have laid out in #1359 to support a clear and more explicit mapping for cursor based pagination is :+1:. It took us a long while to decode and extend the existing relay pagination for our own purposes. The method names, purposes, and the integration with GraphQL were not very clear.

:+1: to batching implementation. In particular, if (:crossed_fingers:) the concurrency abstraction is included in the next release, having a thread-safe batch implementation will be increasingly important. One more thing I would add here, existing implementations of loaders (like graphql-batch) effectively require an entirely new class to be created for each query or network call. This can become very cumbersome, so it would be great to provide a more lightweight way to have concurrent and batch loading lazy objects.

Regarding testing, we have found it difficult to unit test some things (like authorization) for somewhat nested objects because we need to write and execute full queries (instead of simply instantiating the graphql type with a context). Some kinds of helpers or easy ways to instantiate and unit test isolated graphql types would be great. Maybe this is already possible and we just need some examples :grinning:.

Regarding testing, we have found it difficult to unit test some things (like authorization) for somewhat nested objects because we need to write and execute full queries (instead of simply instantiating the graphql type with a context). Some kinds of helpers or easy ways to instantiate and unit test isolated graphql types would be great. Maybe this is already possible and we just need some examples 馃榾.

@panthomakos Not sure if this will help you, but you could implement node and/or nodes fields on Query and then fetch the object(s) you want to test. So then you skip building those big nested queries, but just fetch object(s) of specific type.

Not sure if this will help you, but you could implement node and/or nodes fields on Query and then fetch the object(s) you want to test. So then you skip building those big nested queries, but just fetch object(s) of specific type.

This is a good idea, and I think we will likely have top-level node objects in many APIs, but the downside is that testing forces you to create node objects even if you don't ever want to expose them. Additionally it makes testing in-memory only objects more difficult. By not having to write a query, I can wrap a simple struct or not-yet-persisted ActiveRecord object for speedier testing.

Not sure if this will help you, but you could implement node and/or nodes fields on Query and then fetch the object(s) you want to test. So then you skip building those big nested queries, but just fetch object(s) of specific type.

This is a good idea, and I think we will likely have top-level node objects in many APIs, but the downside is that testing forces you to create node objects even if you don't ever want to expose them. Additionally it makes testing in-memory only objects more difficult. By not having to write a query, I can wrap a simple struct or not-yet-persisted ActiveRecord object for speedier testing.

Here is a method that does not need to expose a public node interface: we could construct a separate anonymous GraphQL schema in tests that subclasses existing schema and overwrites root query object with one for testing.

This is how it can be done:

describe TestTypeClass do
  let(:schema) do
    test_object = { name: 'testing123' }  # create/obtain your test object here
    Class.new(MySchemaClass) do  # subclassing your schema class
      query begin
        Class.new GraphQL::Schema::Object do
          graphql_name 'Query'
          field :testing, TestTypeClass, null: false
          define_method :testing do
            test_object
          end
        end
      end
    end
  end

  let(:result) do
    schema.execute(%|
      {
        testing {
          id
          name
          ...
        }
      }
    |).to_h
  end
  ...

Then we can start writing tests with the above setup:

describe 'field #name' do
  it 'resolves as the value of name attribute' do
    expect(result.dig('data', 'testing', 'name')).to eq('testing123')
  end
end

a proposal that using Ruby accessor style to set graphql_name, description, etc.

e.g change graphql_name 'Query' to self.graphql_name = 'Query' and make graphql_name method a pure reader

@rmosolgo : Thanks for the awesome work 馃帀. I would love to contribute to release 1.10.0.

Instrumentation: remove it

Can I start with this?

Yes, sure! To be clear, it just means removing the docs for field instrumentation. I think query instrumentation has to stay, since there's no alternative (and afaik there doesn't have to be one, it's fine to keep as-is).

@rmosolgo : Should we go ahead and remove this? I think we can remove it unless people need this doc.

Dynamic definition: just remove it?

:+1: sounds good to me. Besides, since the class-based API is _just classes_, you can use plain old Ruby to metaprogram it (Class.new(GraphQL::Schema::Object) { ... }).

(馃殺 to Rubygems :tada:, but better data loading was pushed back 馃槚 )

Was this page helpful?
0 / 5 - 0 ratings