-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix query from AppSync with an empty filter object #61
Fix query from AppSync with an empty filter object #61
Conversation
Build the query/mutation's selection set using the `graphql` utilities, instead of building manually. This will avoid any edge cases with the provided arguments. The schema is assumed to be valid: validation should be taken care of ahead of time in `schemaModelValidator` and we also currently do not define the built-in directives (e.g. `@id`, `@relationship`) required to make it a valid schema. - break out lookup functions for type/field definitions - update `outputReference` resolvers - add unit tests - fix query result tests that were sensitive to object key order (use `toEqual` instead of strict JSON string equality) - also check output file size of `.cjs` resolver in test `Case01.02` - fix running test `Case01.04` (use `.cjs` extension) closes aws#44
Resolve an "unexpected EOF" error from `gql`
(as a result of the resolver changes for queries with an empty filter object)
Hi @danielfinke thanks for all of your recent contributions. This one is a bit more complex than the others so will take a bit more time to review, but I hope to review it this week. |
Hello Daniel, thank you for submitting these changes. The team will look at them soon. |
@andreachild @AndreaNassisi no problem 🙂 I am assembling more PRs as well, no rush |
…ith-an-empty-filter-object
Wrap the JSON in single quotes instead of backticks to reduce the escape sequence surface area. Escape any single quotes in the stringified JSON. This handles single and double quotes and backticks in the input schema's scalar, field/type/etc. descriptions. - add test `Case03.02` to check output for schema with string delimiters - update other `outputReference` resolvers - add templates to lint check - fix resulting ESLint errors in templates - exclude output from ESLint rules (hide errors for those files when using editor integrations) copied changes from aws#61: - fix query result tests that were sensitive to object key order (use `toEqual` instead of strict JSON string equality) - also check output file size of `.cjs` resolver in test `Case01.02` - fix running test `Case01.04` (use `.cjs` extension) closes aws#65
Callers can now pass in GraphQL ASTs directly. Saves a redundant print/parse in `resolveGraphDBQueryFromAppSyncEvent`. For backwards-compatibility, callers *can* continue to pass in a query string. - update `outputReference` resolvers - update tests to pass in ASTs instead of strings see aws#61 (comment)
Callers can now pass in GraphQL ASTs directly. Saves a redundant print/parse in `resolveGraphDBQueryFromAppSyncEvent`. For backwards-compatibility, callers *can* continue to pass in a query string. - update `outputReference` resolvers - update tests to pass in ASTs instead of strings see aws#61 (comment)
LGTM thanks for your patience and taking feedback into consideration |
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.
Thanks @danielfinke, LGTM
Also fix mistyped return annotation on `resolveGraphDBQuery`, my bad
Also fix mistyped return annotation on `resolveGraphDBQuery`, my bad
Issue #, if available: #44
Description of changes:
Build the query/mutation's selection set using the
graphql
utilities, instead of building manually. This will avoid any edge cases with the provided arguments. The schema is assumed to be valid: validation should be taken care of ahead of time inschemaModelValidator
and we also currently do not define the built-in directives (e.g.@id
,@relationship
) required to make it a valid schema.outputReference
resolverstoEqual
instead of strict JSON string equality).cjs
resolver in testCase01.02
Case01.04
(use.cjs
extension)By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.