-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 #1091 #1178
Fix #1091 #1178
Conversation
@@ -127,7 +127,7 @@ export class ValidateCustomFieldsInterceptor implements NestInterceptor { | |||
const map: { [inputName: string]: string } = {}; | |||
|
|||
for (const selection of operation.selectionSet.selections) { | |||
if (selection.kind === 'Field') { | |||
if (selection.kind === 'Field' && !selection.name.value.startsWith('__')) { |
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.
Can we be sure that this will not have unintended consequences? E.g. what if I name my custom field '__mrp'
?
Is there an argument against checking for the full string '__typename'
?
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.
GraphQL is using "__" in various places for internal field.
https://graphql.org/learn/introspection/
or
__resolveType
so I felt to future proof it in case there would be some additions to GraphQL spec in the future
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.
if you prefer just __typename - I will change it
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 just had another thought - perhaps a way to avoid the entire issue with the naming convention is to just make the function ignore an undefined inputType
on line 133, with const arg of inputType?.args ?? []
.
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.
done
Sure will try it and update the PR
On October 20, 2021, GitHub ***@***.***> wrote:
@michaelbromley commented on this pull request.
In packages/core/src/api/middleware/validate-custom-fields-
interceptor.ts <https://github.com/vendure-
ecommerce/vendure/pull/1178#discussion_r732718666>:
@@ -127,7 +127,7 @@ export class ValidateCustomFieldsInterceptor
implements NestInterceptor { const map: { [inputName: string]: string }
= {}; for (const selection of operation.selectionSet.selections) { - if
(selection.kind === 'Field') { + if (selection.kind === 'Field' &&
!selection.name.value.startsWith('__')) {
I just had another thought - perhaps a way to avoid the entire issue
with the naming convention is to just make the function ignore an
undefined inputType on line 133, with const arg of inputType?.args ??
[].
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/vendure-
ecommerce/vendure/pull/1178#discussion_r732718666>, or unsubscribe
<https://github.com/notifications/unsubscribe-
auth/AAKSU3AOYAWSFG2UMGUHV73UH2YCBANCNFSM5GLMEZSQ>.
|
Thank you! |
Fixes #1091