gramps-graphql / data-source-base

Boilerplate for creating a GrAMPS-compatible data source.
https://gramps.js.org/data-source/data-source-overview/
52 stars 8 forks source link

test: Demonstrate issue with dataloader #3

Closed timrs2998 closed 6 years ago

timrs2998 commented 6 years ago

@jlengstorf Attached is example code to demonstrate https://github.com/gramps-graphql/gramps-express/issues/47

Problem

The purpose of the dataloader is to enable per-request caching. If I fetch an item by id "1234" twice in the same request, there should only be one request to the backend. However, fetching an item by id twice results in multiple backend calls.

Example

Start the backend server in one terminal:

$ node server.js
Example app listening on port 3000!

Start the GraphQL server using live data:

$ yarn live-data
..
============================================================
    GrAMPS is running in live mode on port 8080

    GraphiQL: http://localhost:8080/graphiql
============================================================

Submit the following GraphQL request:

query {
  hello:YourDataSource(id:"1234") {
    lucky_numbers
  }
  world:YourDataSource(id:"1234") {
    lucky_numbers
  }
}

And since the dataloader should cache the "1234" entity, I expect only one backend call and I expect the lucky_numbers arrays to be the same, even though lucky_numbers is an array of random numbers.

However, in server.js's logs, I can see two requests:

Request number #0
Request number #1

and in the JSON response:

{
  "data": {
    "hello": {
      "lucky_numbers": [
        7844,
        9953,
        1779,
        7452,
        5061,
        8062,
        7354
      ]
    },
    "world": {
      "lucky_numbers": [
        6489,
        6526,
        1814,
        5528,
        8614,
        4666,
        4819
      ]
    }
  }
}

Solution

I believe the problem is that a new dataloader is constructed every time connector.get() is called, rather than once for every GraphQL request. I'm thinking connector's lifecycle needs to change to accommodate this.

timrs2998 commented 6 years ago

⚠️ don't merge, of course; feel free to close

jlengstorf commented 6 years ago

Issue moved to gramps-graphql/rest-helpers #2 via ZenHub