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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Fieldsthunk to ObjectConfig. #100

Merged
merged 6 commits into from
Jan 15, 2016
Merged

Adds Fieldsthunk to ObjectConfig. #100

merged 6 commits into from
Jan 15, 2016

Conversation

chris-ramon
Copy link
Member

Details.

  • Updates ObjectConfig.Fields to be an interface, so now possibles can be:
    • graphql.Fields & graphql.FieldsThunk
  • Built in top of great work from @pyros2097 on Add FieldsThunk聽#96 馃憤

Note:
I've removed extra code within Object.Fields() since graphql.defineFieldMap() does handle it and is being covered with:
TestTypeSystem_ObjectsMustHaveFields_RejectsAnObjectTypeWithMissingFields

cc/ @pyros2097

@chris-ramon chris-ramon self-assigned this Jan 8, 2016
@chris-ramon chris-ramon added this to the alpha-0.1 milestone Jan 8, 2016
@chris-ramon chris-ramon mentioned this pull request Jan 8, 2016
@pyrossh
Copy link
Contributor

pyrossh commented Jan 9, 2016

Great @chris-ramon I guess coverage has to be improved but other than that I guess we are going to reach the goal of an alpha release soon.

@@ -397,7 +397,10 @@ func doesFragmentConditionMatch(eCtx *ExecutionContext, fragment ast.Node, ttype
if conditionalType == ttype {
return true
}

if conditionalType.Name() == ttype.Name() {
Copy link
Member Author

Choose a reason for hiding this comment

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

@pyros2097 - I might be missing something, do we really should rely only on .Name to say, those are the same types?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess its not absolutely necessary but since every type has to have a unique name this should also hold valid true. It's very useful in because say we have a type ex:
type Quiz struct {} which I use internally I want to be able to send some of its data back to client without creating a new struct so we use a map[string]interface{} and map out the fields we want to send back but the problem arises when using relay.

When we are using nodeDefinition's it doesn't know it is a quiz type but is map[string]interface{}so for this we use the type name like this
Ofcourse we can use any other field but this is easier to reason and it adds some sort of dynamic behavoir to the strong go type struct type system.

MutateAndGetPayload(params graphql.Params) (interface{}, error) {
  return map[string]interface{}{
    "__typename": "Quiz",
    "id": "quiz1",
  }
}

//and in our nodeDefinitions we resolve the type like this
NodeDefinitions = relay.NewNodeDefinitions(relay.NodeDefinitionsConfig{
    IDFetcher: func(id string, info graphql.ResolveInfo) interface{} {
        resolve := relay.FromGlobalID(id)
        switch resolve.Type {
        case "Quiz":
            return map[string]interface{}{
                "__typename": "Quiz",
                "id": "some_id",
            }
            }
    }
  },
            }
  TypeResolve: func(value interface{}, info graphql.ResolveInfo) *graphql.Object {
        switch data := value.(type) {
        case map[string]interface{}:
            switch data["__typename"] {
            case "Quiz":
                return QuizType
            }
        }
    }
})

This helps us in not creating to many structs for a single graphql Type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for clarification @pyros2097 - let's keep this since you've provide a handy use case.

chris-ramon added a commit that referenced this pull request Jan 15, 2016
Adds Fieldsthunk to ObjectConfig.
@chris-ramon chris-ramon merged commit 26c58bd into master Jan 15, 2016
@chris-ramon chris-ramon deleted the fieldsthunk branch January 15, 2016 21:23
@bsr203 bsr203 mentioned this pull request Feb 8, 2016
@andrewscarani
Copy link

@chris-ramon @pyros2097 Any working examples (preferably working test cases) of recursive input types using InputObjectConfigFieldMapThunk?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants