Skip to content
This repository has been archived by the owner. It is now read-only.

New react apollo #275

Merged
merged 11 commits into from Mar 19, 2018

Conversation

Projects
None yet
6 participants
@jbaxleyiii
Copy link
Member

commented Mar 19, 2018

@peggyrayzis this updates the app to use the new Query and Mutation components. Each commit is a separate migration so it should serve as a good example for people looking to update / make the change!

@jbaxleyiii jbaxleyiii merged commit e5d5dc3 into master Mar 19, 2018

3 checks passed

CLA Author has signed the Meteor CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jbaxleyiii jbaxleyiii deleted the new-react-apollo branch Mar 19, 2018

@jedwards1211

This comment has been minimized.

Copy link

commented Apr 3, 2018

Down with HOCs! 😈

@jsierles

This comment has been minimized.

Copy link

commented Sep 24, 2018

What is the motivation for this change? Is the graphql HOC being deprecated?

@DmitryMasley

This comment has been minimized.

Copy link

commented Dec 24, 2018

I like graphql HOC more. Easier to test, easier to reuse. Nested Queries just terrible.

@jedwards1211

This comment has been minimized.

Copy link

commented Dec 28, 2018

@DmitryMasley share some examples that illustrate what's easier. I find render props components easier to refactor with (codemod scripts that wrap a React element in a Query or whatever render props component are simpler than replacing an element with its HOC-ified version), but it would be interesting to see what difficulties render props components cause in your use cases.

@TymchenkoOleksandr

This comment has been minimized.

Copy link

commented Dec 28, 2018

@jedwards1211 , piece of production code with changed names:

compose(
    connect(mapStateToProps, mapDispatchToProps),
    graphql(gql(OPTIONS_QUERY), getSyncOptionsQueryParams("optionsArrayName", "optionsDataKey")),
    graphql(gql(OPTIONS_QUERY), getSyncOptionsQueryParams("optionsArrayName", "optionsDataKey")),
    graphql(gql(OPTIONS_QUERY), getSyncOptionsQueryParams("optionsArrayName", "optionsDataKey")),
    graphql(gql(LOAD_OPTIONS_QUERY), getAsyncOptionsQueryParams("loadOptionsFuncName", "optionsDataKey")),
    graphql(gql(LOAD_OPTIONS_QUERY), getAsyncOptionsQueryParams("loadOptionsFuncName", "optionsDataKey")),
    graphql(gql(LOAD_OPTIONS_QUERY), getAsyncOptionsQueryParams("loadOptionsFuncName", "optionsDataKey")),
    graphql(gql(LOAD_OPTIONS_QUERY), getAsyncOptionsQueryParams("loadOptionsFuncName", "optionsDataKey")),
    graphql(gql(LOAD_OPTIONS_QUERY), getAsyncOptionsQueryParams("loadOptionsFuncName", "optionsDataKey")),
    graphql(gql(SEARCH_QUERY), searchQueryParams),
    graphql(gql(BROWSE_QUERY), browseQueryParams),
    graphql(gql(ENVIRONMENT_QUERY), generateEnvironmentQueryParams()),
    connect(mapPropsToProps)
)(Component);

it is super easy to test and read. it definitely would be an jQuery callback-hell style with your <Query /> and <Mutation /> in jsx approach

@DmitryMasley

This comment has been minimized.

Copy link

commented Dec 28, 2018

@jedwards1211 If you need more than one Query container gets messy.

<Query
    query={query1}
>
    {({data : query1Data, error: query1Error, loading: query1Loading}) => {
        if(query1Error) throw query1Error;
        return <Query
            query={query2}
        >
            {({data: query2Data, error: query2Error, loading: query2Loading}) => {
                if(query2Error) throw query2Error;
                return <Query
                    query={query3}
                >
                    {({data: query3Data, error: query3Error, loading: query3Loading}) => {
                        if(query3Error) throw query3Error;
                        return <Component
                            query1Loading={query1Loading}
                            query2Loading={query2Loading}
                            query3Loading={query3Loading}
                            query1Data={query1Data}
                            query2Data={query2Data}
                            query3Data={query3Data}
                        />
                    }}
                </Query>
            }}
        </Query>
    }}
</Query>

You get ton of variables in one scope and if every query has some business logic there will be more. Every query can't be tested separately. Only way to test this container - mount whole thing and mock responses to all queries. If Component is connected to Redux store or has components inside with other queries you have to mock all that too.
With more queries it looks even worse.

With graphql HOC for each query you likely have options function and props function. These functions work only with data related to single query and easy to test.

If you want to test whole container you can export HOC and test it separately:

const HOC = compose(
    graphql(...),
    graphql(...),
    graphql(...)
)
export default HOC(Component);
export {
    HOC
}

If you mount HOC with some simple test component you have to mock only responses to queries without mocking data for Component

@hwillson

This comment has been minimized.

Copy link
Member

commented Dec 28, 2018

If you prefer using the graphql HOC, great! It's not going anywhere.

Just a quick reminder though, this project is deprecated. If you'd like to continue the HOC vs. Apollo Components discussion, please do so in the Apollo Slack Group. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.