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

Fix get parent field #5

Merged
merged 5 commits into from
May 5, 2016

Conversation

ruslantalpa
Copy link
Contributor

This PR fixes a few bugs and adds missing handling of Union type

@@ -31,13 +31,16 @@ end
function rules.fieldsDefinedOnType(node, context)
if context.objects[#context.objects] == false then
local parent = context.objects[#context.objects - 1]
if(parent.__type == 'List') then
parent = parent.ofType
end
error('Field "' .. node.name.value .. '" is not defined on type "' .. parent.name .. '"')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Field "field" is not defined on type "[Parent]" here would be better (brackets to indicate List type).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see where you are going but i think the message is correct now since "field" is part of the "parent" definition and has nothing to do with the list. For example:
"is_complete" is a property of "task" type sounds good but
"is_complete" is a property of "[task]" type is a bit wired

@bjornbytes
Copy link
Owner

Thanks again! Let me know if you have any questions about my feedback.

@ruslantalpa ruslantalpa force-pushed the fix_get_parent_field branch from 761f37e to 7ba7b5f Compare April 30, 2016 09:38
@ruslantalpa
Copy link
Contributor Author

Your suggestions implemented. Waiting for this to be merged in order to continue with introspection and fixing some "missing checks/defaultTypeResolve" in relation to unions and interfaces

@bjornbytes bjornbytes merged commit 5235df4 into bjornbytes:master May 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants