Promise Namespacing Issue

[jpserra]

There is a Namespacing inconsistency in GraphQL::Batch::Loader between the Promises returned by load and load_many causing them to return different classes.

   def load(key)
      cache[cache_key(key)] ||= begin
        queue << key
        Promise.new.tap { |promise| promise.source = self } ### returns GraphQL::Batch::Promise
      end
    end

    def load_many(keys)
      ::Promise.all(keys.map { |key| load(key) })  ### returns Promise 
    end

Is there a particular reason for this? I can't seem to reason one.

[dylanahsmith]

Your issue doesn't actually explain a problem. If you are doing something like .is_a?(GraphQL::Batch::Promise) in your application code, change it to .is_a?(Promise)

The reason for the difference is more of an implementation detail. The only difference between ::Promise and GraphQL::Batch::Promise is that GraphQL::Batch::Promise overrides defer so that we can use GraphQL::Batch::Executor.current.loading in tests to detect when an unbatched query is performed in a test.

[jpserra]

That's exaclty my problem, but it didn't seem resonable to have to validate for both Promise and GraphQL::Batch::Promise since I want the code to work both when i call load and load_many seemlessly.

[dylanahsmith]

I don't know what is making it hard to write code that works for both Promise and GraphQL::Batch::Promise objects. Just don't reference GraphQL::Batch::Promise or do class equality checks.

Doing so will also allow Promise.all(promises) to just work, so you don't need to use the more verbose GraphQL::Batch::Promise.all(promises).

Ideally, I would like to just return Promise objects from both load and load_many in the future, deprecate GraphQL::Batch::Promise and alias it to ::Promise. However, this hasn't been a high priority, since it hasn't been causing problems for us that I know of.