Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Compose function not working as expected #315

Closed
ryannealmes opened this issue Nov 7, 2016 · 22 comments
Closed

Compose function not working as expected #315

ryannealmes opened this issue Nov 7, 2016 · 22 comments

Comments

@ryannealmes
Copy link

Using the compose function I ran into a problem where props were not being passed to my connected component state changes.

I was using the compose function below

const mapStateToProps = (state) => ({
  filtering: state.filtering,
  sorting: state.sorting
});

export default compose(
  graphql(query, config),
  connect(mapStateToProps, mapDispatchToProps)
)(PeopleContainer);

after changing to

I was using the compose function below

const mapStateToProps = (state) => ({
  filtering: state.filtering,
  sorting: state.sorting
});

export default compose(
  connect(mapStateToProps, mapDispatchToProps),
  graphql(query, config)
)(PeopleContainer);

I think some things required to reproduce might be related to my setup.

  • I am using redux
  • it's integrated with the apollo client

"apollo-client": "0.5.0",
"react-apollo": "0.5.16",
"graphql-tag": "0.1.15",

This might not be a bug, but I thought I would mention it since the documentation connects the redux store after the graphql query and mutations. If this intended, it may be worth mentioning in the docs that order matters.

http://dev.apollodata.com/react/higher-order-components.html#

TimMikeladze added a commit to TimMikeladze/react-docs that referenced this issue Nov 7, 2016
@stubailo
Copy link
Contributor

stubailo commented Nov 8, 2016

This is surprising, I would have expected it to work both ways. I wonder if perhaps it's something to do with react-redux. Can you create a reproduction app or failing test we can try out?

@ryannealmes
Copy link
Author

I don't have time right now, have a long flight today, but I will see what I can do in the next couple of days.

@stefanorg
Copy link

I had the same issue, a very simple component with a search input text and a tabular data filtered by the search input.

// @flow
import React, { Component } from 'react';
import Home from '../components/Home';
import { graphql, compose } from 'react-apollo';
import { connect } from 'react-redux';
import gql from 'graphql-tag';

const TestQuery = gql`
  query test($query: String) {
    dispositivi (search: {q:$query}){
      id,
      nome,
      seriale
    }
  }
`;

const queryOptions = {
  props: ({ ownProps, data }) => ({
    loading: data.loading,
    dispositivi: data.dispositivi
  }),
  options(props) {
    console.log("queryOptions props", props);
    return {
      variables: {
        query: props.searchQuery
      }
    };
  },
};

const HomePageWithDataAndState = compose(
  graphql(TestQuery, queryOptions), //<--THIS IS WRONG
  connect(
    // stateToProps
    (state) => ({
      searchQuery: state.search.query
    })
  ),
)(Home);

export default HomePageWithDataAndState;

I had to put graphql(TestQuery, queryOptions) at the bottom as pointed by @ryannealmes

const HomePageWithDataAndState = compose(
  connect(
    // stateToProps
    (state) => ({
      searchQuery: state.search.query
    })
  ),
graphql(TestQuery, queryOptions), //<--THIS IS GOOD
)(Home);

At this point the client is aware of the changed searchQuery prop once the redux action is dispatched and the apollo middleware take care of re run the query.

@abhiaiyer91
Copy link
Contributor

https://github.com/reactjs/redux/blob/master/docs/api/compose.md

Compose is just like lodash flowRight. So functions will read right to left or top to bottom however visually you write your HOC.

I think we can close this issue

@advance512
Copy link

advance512 commented Mar 16, 2017

Why does the order being left to right or right to left matter?
According to the documentation:

You can connect before or after (or both!) attaching GraphQL data to your component with graphql

This is obviously not true if what is reported above occurs.

In fact I'm having the same issue the previous posters reported - only worse. I can't get react-apollo and react-redux to work together with the same component.

import { graphql, compose } from 'react-apollo';
import { connect } from 'react-redux';

...
...

const enhance = compose(
  graphql(ViewerAccountsQuery, {
    props: mapViewerAccountsQueryResultsToProps,
  }),
  graphql(IdentityDetailsQuery, {
    props: mapIdentityDetailsQueryResultsToProps,
  }),
  connect(mapStateToProps, mapDispatchToProps),
);

export const AccountVerifier = enhance(AccountVerifierControl);

This doesn't work. Nor does putting connect() as the first item in the compose() list. Following mapStateToProps (even an empty one, that just returns {}), the other HOC functions aren't called.

Not sure what the issue is, to be honest, but it seems like it is indeed with react-redux and not react-apollo.

@helfer
Copy link
Contributor

helfer commented Mar 21, 2017

Could one of you provide a reproduction of this with https://github.com/apollographql/react-apollo-error-template please? Thanks! 🙂

@helfer helfer closed this as completed Mar 21, 2017
@helfer helfer reopened this Mar 21, 2017
@advance512
Copy link

I think the issue I encountered was having multiple versions of redux or react-redux installed. Non deterministic NPM FTW.

@deilv
Copy link

deilv commented Aug 2, 2017

This issue still exists in react-apollo 1.4.10.

The code below fails.

export const viewerQuery = gql`
    query viewer($token: String) {
        viewer(token: $token) {
            username
        }
    }
`;

export const viewerQueryOptions = {
    options: ({ token }) => ({
        variables: { token },
        fetchPolicy: 'network-only'
    })
};

export default compose(
    graphql(viewerQuery, viewerQueryOptions),
    connect(mapStateToProps, mapDispatchToProps)
)(Component);

The code works as intended if I make this change:

export default compose(
    connect(mapStateToProps, mapDispatchToProps),
    graphql(viewerQuery, viewerQueryOptions)
)(Component);

However, according to the documentation this shouldn't be the case. connect() should be called first (to map state to props) and then be available to graphql(). I will investigate further, but perhaps this should be reopened.

@advance512
Copy link

Did you
yarn list | grep redux
?

@deilv
Copy link

deilv commented Aug 3, 2017

Yes, redux is properly installed and I even checked the content of the compose right before it's used (compose.toString()) and after further testing I am convinced this is not a problem with compose.

The fact is that even though it does not matter in which order you place functions in compose (mentioned in the documentation), it does matter if one function depends on another. If graphql() has variables that require props from connect, then connect() must be placed above graphql(), so the result of compose() will be connect(graphql(component)).

If someone else can confirm this, perhaps it should be added as a note in the documentation.

@advance512
Copy link

Compose is a really simple function, and its implementation (whether from react-apollo, redux or recompose) I doubt is faulty. But you can try all 3. Does any of them work while the others does not?

I am nearly certain the issue is multiple co-existing versions of Redux. Can you try:

yarn install --flat

and tell us what happens? Does it solve the issue?

@deilv
Copy link

deilv commented Aug 9, 2017

At this point I am quite certain that this is not an issue with compose, at least not in my case. yarn install --flat makes no difference. However, I did notice that in the documentation example (http://dev.apollodata.com/react/redux.html#using-connect) connect() wraps graphql(). I firmly believe that connect() should be before graphql() in compose() when your query needs variables from the store. TBH it makes sense if you consider the variable scope when compose() builds nested functions.

@andrekovac
Copy link

andrekovac commented Oct 12, 2017

@deilv Could you please elaborate on the variable scopes. I'm still confused with the order and would believe that graphql call should wrap the connect call, so that connect is called first to get the data which can then be used in the graphql part.

@deilv
Copy link

deilv commented Oct 12, 2017

@andruschenko The thing you need to understand is that both functions inside compose are actually factory functions and return a function which is then called. The outside function can provide context to the inside function. This means that connect() provides state and dispatch methods to graphql().

Note, however, that this only matters if graphql requires connect data for its parameters (store state) to initialize, like in my example. If it does not, then you can call them in any order you want, because all functions within compose will always be available to the original component, which is provided as raw material to the factory.

@andrekovac
Copy link

@deilv Thanks for your reply! So does it work like this?: connect() and graphql() are both called independently with their respective arguments (e.g. mapStateToProps and mapDispatchToProps in the case of redux). So then when connect(graphql(MyComponent)) is called with MyComponent the selected state is provided to the component returned by graphql(MyComponent) via props.

Is that somewhat correct?

@deilv
Copy link

deilv commented Oct 13, 2017

@andruschenko not quite.

  1. Compose() is called with 2 parameters: the result of calling connect(mapStateToProps, mapDispatchToProps) and the result of calling graphql(query, options). Both of those parameters are functions, not components. The result is a composition of all functions, which is a new function itself, not a component.
  2. The composed function is called with the component as a parameter and returns the final component. All previous parameters (mapStateToProps, mapDispatchToProps, query, options) are used within the composed function like properties are used in a class.

I'm sorry if this doesn't make it clear enough for you, but this GitHub issue isn't the best place for this discussion. Perhaps you can seek further assistance in stackoverflow or just take my word for it and use the ordering I suggested. :)

@TSMMark
Copy link
Contributor

TSMMark commented Oct 13, 2017

Hey compose experts, anyone know how to annotate a compose block with flow type? I found a simple example here and it works but it's not nearly as concise as compose.

We do happen to import compose at the top of the react-apollo flow test, but it's unused.

Much appreciated!

@SpencerCDixon
Copy link

@TSMMark I don't see any examples using flow + compose. Any chance you have a link you could point me to?

@TSMMark
Copy link
Contributor

TSMMark commented Dec 7, 2017

@SpencerCDixon the best I could do was

  1. type as many of the HOCs as possible.
  2. type the variable that gets returned from compose.
type OwnProps = {
  foo: string
}

type StateProps = {
  somethingFromRedux: string
}

type Props = 
  & OwnProps
  & StateProps

const withQuery: OperationComponent<myQuery, OptionDescription<StateProps>, Props> = graphql(query, queryOptions)

const connected: React$ComponentType<OwnProps> = compose(
  connect(mapStateToProps, mapDispatchToProps),
  withQuery,
  injectSheet(styles),
  withWhateverElse,
)(MyComponent)

Not sure if there's a better way but this does work.

@SpencerCDixon
Copy link

ahhh, I see. Thank you so much for the snippet!! 😄

@Madgeniusblink
Copy link

Madgeniusblink commented Nov 3, 2019

Hi,

I keep getting a similar issue: "undefined is not a function (near '...(0, _reactApollo.compose)...')"
the difference is that I am not using redux. I am only using Apollo and react Native.

The issue keeps telling me to look at:

export default compose(
    graphql(signinUser, { name: "signinUser"}),
    graphql(createUser, { name: "createUser"})
)(CreateUser);

has anyone encounter similar issue? I know for a fact the issue is on compose but no idea how to fix

@dylanwulf
Copy link
Contributor

@Madgeniusblink The compose utility was recently removed from react-apollo (check the changelog for v3.0.0

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

No branches or pull requests