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
WIP: Reduce the number of SQL queries #342
Changes from all commits
8dc0b9b
c390862
cd22083
b614cf3
6d94bf7
3a85c3b
e4f3939
c266056
4b1208c
b9ebaea
0ba7e1f
8435b27
59e05b3
9738fcf
e25b80d
16a5129
bc8ec2b
e6668f3
48865dd
b4c3606
8822038
a9d0b81
6cf1af6
f6bf5f0
6e7819f
a2c8c97
12f9428
47d8f9b
e3ebd86
5bb3898
9a5c720
948200f
fbcee96
7a14a42
c0ce393
8ad4521
051ac2d
94061d7
af230a0
69e142d
bfda962
94d0898
34fcd85
5444de6
a3dc29a
2b4195e
54091ee
8d08073
3808e37
a9b5f45
5b01610
4276920
3ca379c
bedc861
3df8863
9a4ec44
2e747e6
878368b
997a165
a8bae58
9d869fe
814008e
09fd1f1
edd133e
76a9208
fb40c7d
11a0a96
880e77b
6d1a314
55d038e
cd09cc9
2fa1b05
e5e4e58
f51c9bc
f8f68ef
f45e45c
63f55a3
9e358cc
384da26
333f513
d1325c9
3304343
e9d8035
387d5f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import createConnectionGqlField from '../connection/createConnectionGqlField' | |
import BuildToken from '../BuildToken' | ||
import createCollectionRelationTailGqlFieldEntries from './createCollectionRelationTailGqlFieldEntries' | ||
import getConditionGqlType from './getConditionGqlType' | ||
import { isSymbol } from 'lodash' | ||
|
||
/** | ||
* Creates the output object type for a collection. This type will include all | ||
|
@@ -37,6 +38,7 @@ export default function createCollectionGqlType<TValue> ( | |
primaryKey && [options.nodeIdFieldName, { | ||
description: 'A globally unique identifier. Can be used in various places throughout the system to identify this single value.', | ||
type: new GraphQLNonNull(GraphQLID), | ||
externalFieldNameDependencies: primaryKey._keyTypeFields.map(([fieldName, field]) => field.externalFieldName), | ||
resolve: (value: TValue) => idSerde.serialize(collection, value), | ||
}], | ||
], | ||
|
@@ -50,6 +52,7 @@ export default function createCollectionGqlType<TValue> ( | |
value: { | ||
description: field.description, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing "," in line 52 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, good catch. It seems the TypeScript compiler (at least the version I'm using) is hella forgiving. It's also really REALLY hard to figure out where syntax has gone wrong with TypeScript as compared to Babel which is so much more helpful with its errors. |
||
type: gqlType, | ||
externalFieldName: field.externalFieldName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we can’t have just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why not. |
||
resolve: (value: TValue): mixed => intoGqlOutput(field.getValue(value)), | ||
}, | ||
} | ||
|
@@ -93,11 +96,15 @@ export default function createCollectionGqlType<TValue> ( | |
], | ||
// We use the config when creating a connection field to inject | ||
// a condition that limits what we select from the paginator. | ||
getPaginatorInput: (headValue: TValue, args: { condition?: { [key: string]: mixed } }) => | ||
getPaginatorInput: (sourceOrAliasIdentifier: mixed, args: { condition?: { [key: string]: mixed } }) => | ||
conditionHelpers.and( | ||
relation.getTailConditionFromHeadValue!(headValue), | ||
isSymbol(sourceOrAliasIdentifier) | ||
? relation.getTailConditionFromHeadAlias!(sourceOrAliasIdentifier) | ||
: relation.getTailConditionFromHeadValue!(sourceOrAliasIdentifier), | ||
conditionFromGqlInput(args.condition), | ||
), | ||
subquery: true, | ||
relation, | ||
}), | ||
] | ||
}), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,11 +46,12 @@ export default function createCollectionRelationTailGqlFieldEntries <TSource, TV | |
: formatName.field(`${headCollection.type.name}-by-${relation.name}`), | ||
{ | ||
description: `Reads a single ${scrib.type(headCollectionGqlType)} that is related to this ${scrib.type(collectionGqlType)}.`, | ||
externalFieldNameDependencies: relation._tailFieldNames, // 🔥 Needs to account for classicIds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to account here for classicIds? The source of a relation is a column != id , it does not seem to address the destination of the relation here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's possible that your primary key is called "flibbledeedee" and that your "id" column is merely a foreign key. |
||
type: headCollectionGqlType, | ||
async resolve (source: TSource, _args: {}, context: mixed): Promise<mixed> { | ||
async resolve (source: TSource, _args: {}, context: mixed, resolveInfo: mixed): Promise<mixed> { | ||
const value = options.getCollectionValue(source) | ||
const key = relation.getHeadKeyFromTailValue(value) | ||
const headValue = await headCollectionKey.read!(context, key) | ||
const headValue = await headCollectionKey.read!(context, key, resolveInfo, headCollectionGqlType) | ||
|
||
if (headValue == null) | ||
return | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
primaryKey._keyTypeFields
depends on an internal implementation detail, right? Any reason why we couldn’t just do:externalKeyDependencies: [primaryKey]
. This would also work for some relations.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know where to grab this from, so just grabbed the nearest available thing. Also what if it's a compound primary key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
external describes the database side or is external the graphql side?