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

Inconsistent state in mapMutationsToProps #41

Closed
stubailo opened this issue May 3, 2016 · 6 comments
Closed

Inconsistent state in mapMutationsToProps #41

stubailo opened this issue May 3, 2016 · 6 comments

Comments

@stubailo
Copy link
Contributor

stubailo commented May 3, 2016

Originally posted by @deoqc on apollo-client:


I am using redux-form and have 2 forms (lets say FormA and FormB, 'living' in state.form.FormA and state.form.FormB).

The problem is that FormB is not always showing in the state when using mapMutationsToProps, but it is correct in mapStateToProps or in the store (as shown by devtools).


To be more precise, my code is something like this:

const mapMutationsToProps = ({ state }) => ({
  onMutation: () => ({
    mutation: `
      mutation myAwesomeMutation (
        $formKeys: String!
      ) {
        awesomeMutation( 
          formKeys: $formKeys
        ) {
          passThroughtInput
        }
      }
    `,
    variables: {
      formKeys: JSON.stringify(Object.keys(state.form)),
    },
  })
});

and the mutation only returns the same input (passThroughtInput === formKeys)

To check, I also have the following:

const mapStateToProps = (state) => ({
  formKeys: JSON.stringify(Object.keys(state.form)),
});

The callback to the onMutation prints the results on console, and the formKeys is in a Text tag on screen, and also using devtools (so I compare the 3).

As said before, mapStateToProps and store devtools are correct. Mutation is missing (at least most of the time) the state.FormB key (but showing state.FormA).

If you guys don't have any clues, I can try to do a reproducible repo.

@jbaxleyiii
Copy link
Contributor

@stubailo @deoqc This happens because mapMutationsToProps is called on render of the child component, but is not called on subsequent state changes. I'll work on updating it to support state changes to rebuild the mutation action.

Will let you know when it is improved!

@stubailo
Copy link
Contributor Author

Hmm, perhaps we could instead just use the current store state? Rather than initializing a function that includes the state?

@jbaxleyiii
Copy link
Contributor

@stubailo we could pass a method to get the state which would give more dynamic options:

Option 1

const mapMutationsToProps = ({ getState }) => ({
  onMutation: () => ({
    mutation: `
      mutation myAwesomeMutation (
        $formKeys: String!
      ) {
        awesomeMutation( 
          formKeys: $formKeys
        ) {
          passThroughtInput
        }
      }
    `,
    variables: {
      formKeys: JSON.stringify(Object.keys(getState().form)),
    },
  })
});

Option 2

const mapMutationsToProps = ({ getState }) => {
  const state = getState()
  if (state.boolValue) {
   return { // other mutation }
  }
 return {
  onMutation: () => ({
    mutation: `
      mutation myAwesomeMutation (
        $formKeys: String!
      ) {
        awesomeMutation( 
          formKeys: $formKeys
        ) {
          passThroughtInput
        }
      }
    `,
    variables: {
      formKeys: JSON.stringify(Object.keys(getState().form)),
    },
  })
 }
};

@stubailo
Copy link
Contributor Author

Hmm, this seems a bit odd and kinda encourages people to start doing interesting stuff with getState which ruins the "declarative-ness" of the current API. Perhaps we should just rebuild the mutations.

@deoqc
Copy link

deoqc commented May 11, 2016

@jbaxleyiii I can think in some use cases - like signup vs login w/ username vs login w/ email - that I would be tempted to put the logic there like the Option 2. But I think should be better to just have a function that do these logic and call the correct static mutation.

@stubailo Vote for keeping the initial proposed API mapMutationsToProps = ({ state, ownProps, ...}) but with the current state for sure.

Also, the getState or state discussion should be orthogonal w/ having a dynamic mutation like Option 2, no?

@jbaxleyiii
Copy link
Contributor

@deoqc what are your thoughts around #51?

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

3 participants