Skip to content

Use of map() where parent type is a node #6

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

Closed
johanobergman opened this issue Oct 9, 2015 · 3 comments
Closed

Use of map() where parent type is a node #6

johanobergman opened this issue Oct 9, 2015 · 3 comments

Comments

@johanobergman
Copy link

Resolving fields with map works really well (and fast!) when the parent type is a list. However, when using Relay connections, it's impossible to use map on the "connected" type, since its parent is an edge and not the list of edges.
An example:
UserType -> todos -> TodoConnection -> edges -> EdgeType -> node -> TodoType -> items*
I want to use map on items but I cant since the root argument will contain an array with only one element - the edge.

Is there any way to work around this, or is it perhaps something you would consider improving?

@vladar
Copy link
Member

vladar commented Oct 11, 2015

Hey, nice to see that you tried to use map :)

This feature was trying to solve the N+1 problem in generic way, so that you could do bulk operations, like Redis MGET or SQL IN(...).

But as you discovered map doesn't work well with some use-cases like Relay connections, when item is wrapped with some other type (there are also other cases where it doesn't work).

So at this point I can say that this feature failed to solve original problem. And I will probably revert it until better generic solution reveals itself.

Other GraphQL implementations also discuss this problem. Check out graphql/graphql-js#111 and graphql-dotnet/graphql-dotnet#21 for example.

For now you can work around it in the user-land with regular resolve by preserving original list together with value and resolving field for all items in list on first resolve call. For example:

class Value {
    /** @var Value[] */
    public $parentList;
}

Then in your resolve:

'fieldName' => [
    'resolve' => function (Value $value, $args, $context) {
        if (!isset($value->fieldName) {
            // Resolve field for all items in list on first resolve call:
            $list = $value->parentList ?: [$value];
            foreach ($list as $item) {
                $item->fieldName = 'something';
            }
        }
        return $value->fieldName;
    }
]

This is way more ugly than map, but it does work in all cases.

I realize that N+1 problem is severe (especially in pure PHP world without async operations) and it would be great to have some clean and expressive solution on library level instead of user-land hacks, but for now, map is just not the way to go.

When good-enough solution will be ready - I will add separate section to README for it. If you have any thoughts on how to make it work for all use-cases - please share your thoughts!

@johanobergman
Copy link
Author

Thanks for the comment!

It seems like it's a rather tough problem to tackle - there really should be a standardized solution to this in GraphQL. For now, your hack works pretty well and solved my problems.

I've even experimented with passing down a query builder instance (from my ORM) alongside the parent list - this actually makes it possible to perform JOINs instead of IN(...)s.

@vladar
Copy link
Member

vladar commented Oct 23, 2015

Closing this as map is reverted now. Will add note in README when alternative is ready.

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

No branches or pull requests

2 participants