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

Add Promise#spawn. #35

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arthurschreiber
Copy link
Contributor

This is a new internal method that is used by Promise#then to create a new Promise. This method can be overriden by Promise subclasses to have finer grained control about the promise type returned by chaining.

The main target for this method are libraries like graphql-batch, which come with their own Promise sub-class with a custom defer method. In the case of graphql-batch, defer is overriden to set the current executor's loading attribute to false (via executor.defer). Unfortunately, this means that executor.defer is called for each chained promise as well.

In the following snippet, there's a total of 4 calls to executor.defer:

require "graphql/batch"

executor = GraphQL::Batch::Executor.new

GraphQL::Batch::Executor.current = executor

p1 = GraphQL::Batch::Promise.new
puts p1.class #=> GraphQL::Batch::Promise

p2 = p1.then do
  executor.loading #=> "false"
end
puts p2.class #=> GraphQL::Batch::Promise

p3 = p2.then do
  executor.loading #=> "false"
end
puts p2.class #=> GraphQL::Batch::Promise

p4 = p3.then do
  executor.loading #=> "false"
end
puts p2.class #=> GraphQL::Batch::Promise

p5 = p4.then do
  executor.loading #=> "false"
end
puts p5.class #=> GraphQL::Batch::Promise

executor.send(:with_loading, true) do
  p1.fulfill(:foo)
end

By implementing a custom GraphQL::Batch::Promise#spawn that returns Promise instances instead of GraphQL::Batch::Promise instances, we can get the calls to executor.defer down to 1, just for the first promise in the chain:

require "graphql/batch"

class GraphQL::Batch::Promise
  protected

  def spawn
    ::Promise.new
  end
end

executor = GraphQL::Batch::Executor.new

GraphQL::Batch::Executor.current = executor

p1 = GraphQL::Batch::Promise.new
puts p1.class #=> GraphQL::Batch::Promise

p2 = p1.then do
  executor.loading #=> "false"
end
puts p2.class #=> Promise

p3 = p2.then do
  executor.loading #=> "false"
end
puts p2.class #=> Promise

p4 = p3.then do
  executor.loading #=> "false"
end
puts p2.class #=> Promise

p5 = p4.then do
  executor.loading #=> "false"
end
puts p5.class #=> Promise

executor.send(:with_loading, true) do
  p1.fulfill(:foo)
end

This is a new internal method that is used by `Promise#then` to create a new Promise. This method can be overriden by `Promise` subclasses to have finer grained control about the promise type returned by chaining.
@dylanahsmith
Copy link
Collaborator

Did Shopify/graphql-batch#65 remove this need for this PR?

@theojulienne
Copy link

Did Shopify/graphql-batch#65 remove this need for this PR?

This PR would still be very useful for other use cases where a Promise is specialised in some way, but chained promises don't need to be (so would be best if they were normal Promise base class instances rather than a subclass). Currently subclasses need to bypass functionality for these spawned chained promises, but it would be much cleaner if they could just prevent chaining using the subclass.

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

Successfully merging this pull request may close these issues.

None yet

3 participants