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

getConnectionResolver Improvements #1

Open
3 tasks
smartprix opened this issue Aug 22, 2017 · 4 comments
Open
3 tasks

getConnectionResolver Improvements #1

smartprix opened this issue Aug 22, 2017 · 4 comments

Comments

@smartprix
Copy link
Collaborator

smartprix commented Aug 22, 2017

  • totalCount should use nested query
  • totalCount should remove all order by statements (look into objection native API, eg. resultSize)
  • totalCount should remove runAfter and maybe runBefore
@rohit-gohri
Copy link
Contributor

rohit-gohri commented Jan 18, 2019

  • last paging argument also doesn't work with getConnectionResolver
The first argument must be one of type string, Buffer, ArrayBuffer, Array, or Array-like Object. Received type undefined
TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be one of type string, Buffer, ArrayBuffer, Array, or Array-like Object. Received type undefined
    at Function.from (buffer.js:208:11)
    at getIdFromCursor (/home/smpx-170l/projects/smartprix/node_modules/gqutils/dist/connection.js:13:24)
    at getPagingParams (/home/smpx-170l/projects/smartprix/node_modules/gqutils/dist/connection.js:37:18)
    at getConnectionResolver (/home/smpx-170l/projects/smartprix/node_modules/gqutils/dist/connection.js:85:7)
    at categories (/home/smpx-170l/projects/smartprix/src/lib/Category/resolvers.js:71:22)
    at /home/smpx-170l/projects/smartprix/node_modules/graphql-tools/src/makeExecutableSchema.ts:102:20
    at /home/smpx-170l/projects/smartprix/node_modules/graphql-tools/src/generate/decorateWithLogger.ts:32:22
    at resolveFieldValueOrError (/home/smpx-170l/projects/smartprix/node_modules/graphql/execution/execute.js:479:18)
    at resolveField (/home/smpx-170l/projects/smartprix/node_modules/graphql/execution/execute.js:446:16)
    at executeFields (/home/smpx-170l/projects/smartprix/node_modules/graphql/execution/execute.js:293:18)
    at executeOperation (/home/smpx-170l/projects/smartprix/node_modules/graphql/execution/execute.js:237:122)
    at executeImpl (/home/smpx-170l/projects/smartprix/node_modules/graphql/execution/execute.js:85:14)
    at Object.execute (/home/smpx-170l/projects/smartprix/node_modules/graphql/execution/execute.js:62:229)
    at doRunQuery (/home/smpx-170l/projects/smartprix/node_modules/apollo-server-core/src/runQuery.ts:197:7)
    at /home/smpx-170l/projects/smartprix/node_modules/apollo-server-core/src/runQuery.ts:80:39

@rohit-gohri
Copy link
Contributor

rohit-gohri commented Mar 2, 2019

  • hasNextPage should also use totalCount result instead of just checking edgeCount and limit. Currently it returns wrong results when limit is set to totalCount

@hit12369
Copy link
Member

hit12369 commented Mar 2, 2019

totalCount can be very expensive. Checking edgeCount is just a way to have a no cost hasNextPage, this might be wrong in the last page of results, but I don't think it's that big of an issue (next page will have empty results and hasNextPage will be false).

@rohit-gohri
Copy link
Contributor

We can at least do something like cachetotalCount (like nodes are) and use it if both totalCount and hasNextPage are queried.
Otherwise this should be mentioned in the readme or in the description of the hasNextPage field.

rohit-gohri added a commit that referenced this issue Apr 30, 2019
@rohit-gohri rohit-gohri mentioned this issue May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants