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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot read property 'type' of undefined when no function is explicitly returned on the action creator #29

Closed
andre0799 opened this issue Nov 5, 2015 · 13 comments

Comments

@andre0799
Copy link

Why does I get this error when I implement the action without the return function (and which is how @gaearon told me I should do here reduxjs/redux#994 (comment) )

export function createPlan() {
  return dispatch => {
    const planId = uuid();
    const exerciseId = uuid();

    dispatch({
      type: 'CREATE_PLAN',
      plan: {title: 'Treino A', exercises: [exerciseId], id: planId},
      id: planId
    });

    // return function(){
      dispatch({
        type: 'CREATE_EXERCISE',
        id: exerciseId,
        exercise: {
          id: exerciseId,
          title: 'CREATE_PLAN'
        }
      });
    // }
  };
}

image

but when I uncomment the return function it works?

I'm calling this action for the view, like this this.props.dispatch(this.actions.createPlan(id))

I'm missing something here?

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

If you use DevTools with middleware, make sure you apply them in correct order as mentioned in DevTools README.

Inside compose() call, applyMiddleware() should go first, and DevTools call should go last.

@gaearon gaearon closed this as completed Nov 5, 2015
@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

Maybe this isn't it though. You'll need to show your DevTools integration code because something is wrong there.

@andre0799
Copy link
Author

I'm applying the middleware first...

import React from 'react';  
import { createStore as initialCreateStore, compose, applyMiddleware } from 'redux';
import thunkMiddleware from 'redux-thunk';

import createLogger from 'redux-logger';

const logger = createLogger({collapsed: true});

export let createStore = initialCreateStore;

if (__DEV__) {  
  createStore = compose(
    applyMiddleware(thunkMiddleware, logger),
    require('redux-devtools').devTools(),
    require('redux-devtools').persistState(
      window.location.href.match(/[?&]debug_session=([^&]+)\b/)
    )
  )(createStore);
}

export function renderDevTools(store) {  
  if (__DEV__) {
    let {DevTools, DebugPanel, LogMonitor} = require('redux-devtools/lib/react');
    return (
      <DebugPanel top right bottom>
        <DevTools store={store} monitor={LogMonitor} />
      </DebugPanel>
    );
  }
  return null;
}

@andre0799
Copy link
Author

And then on index.js I'm doing like this

import React from 'react';
import ReactDOM from 'react-dom';
import createHistory from 'history/lib/createHashHistory';
// import createHistory from 'history/lib/createBrowserHistory';

import { Provider } from 'react-redux';
import { Router, Redirect } from 'react-router';
// import configureStore from './store/configureStore';
import routes from './routes';

import reducer from './reducers/index.js';

import { createStore, renderDevTools } from './utils/devTools';

// const store = configureStore();

const store = createStore(reducer);  

console.log(store.getState())

ReactDOM.render(
  <div>
    <Provider store={store}>
      <Router history={createHistory()}>
        <Redirect from="/" to="home" />
        {routes}
      </Router>
    </Provider>
    {renderDevTools(store)}
  </div>,
  document.getElementById('root')
);

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

Can you please create a sample project reproducing the problem?

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

While unrelated, you don't seem to apply the middleware in production mode.
It should be like this instead:

export let createStore = initialCreateStore;

if (__DEV__) {  
  createStore = compose(
    applyMiddleware(thunkMiddleware, logger),
    require('redux-devtools').devTools(),
    require('redux-devtools').persistState(
      window.location.href.match(/[?&]debug_session=([^&]+)\b/)
    )
  )(createStore);
} else {
  // I added this:
  createStore = applyMiddleware(thunkMiddleware)(createStore);
}

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

What is simple?
It kinda looks like your reducer is crashing, reading type of something that doesn't exist.

@andre0799
Copy link
Author

simple was a reducer I was not using anymore, but that was not the problem, I removed it and added the project to this temp repository https://github.com/andre0799/workoutTable-redux-experiment/tree/master

thanks a lot @gaearon

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

Here's the problem:

@connect(state => state.workout)
export class WorkoutTable extends Component {

  constructor(props) {
    super(props);
    this.actions = bindActionCreators(actionCreators, this.props.dispatch);
  }

  onAddPlan = () =>{
    const {currentPlans} =  this.props
    this.props.dispatch(this.actions.createPlan())
  }

bindActionCreators is a shortcut to help you write concise code by not calling dispatch yourself.

// this is wrong!
let actions = bindActionCreators(actionCreators, this.props.dispatch);
dispatch(actions.createPlan())

// because it is equivalent to this: (also wrong)
dispatch(dispatch(actions.createPlan()))

Instead, you need to write

// this is right!
let actions = bindActionCreators(actionCreators, this.props.dispatch);
actions.createPlan()

// because it is equivalent to this: (also right)
// note: I didn't use bindActionCreators at all
dispatch(actionCreators.createPlan())

So, your particular example is fixed by changing the code to not call bindActionCreators—or to keep the call, but not call dispatch().

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

To get back at your specific example:

The Problematic Code

@connect(state => state.workout)
export class WorkoutTable extends Component {

  constructor(props) {
    super(props);
    this.actions = bindActionCreators(actionCreators, this.props.dispatch);
  }

  onAddPlan = () =>{
    const {currentPlans} =  this.props
    this.props.dispatch(this.actions.createPlan())
  }

Possible Fix 1: Do Not Call dispatch()

@connect(state => state.workout)
export class WorkoutTable extends Component {

  constructor(props) {
    super(props);
    this.actions = bindActionCreators(actionCreators, this.props.dispatch);
  }

  onAddPlan = () =>{
    const {currentPlans} =  this.props
    this.actions.createPlan()
  }

Possible Fix 2: Do Not Use bindActionCreators and Keep dispatch Call

@connect(state => state.workout)
export class WorkoutTable extends Component {
  onAddPlan = () =>{
    const {currentPlans} =  this.props
    this.props.dispatch(actionCreators.createPlan())
  }

Possible Fix 3: Let React Redux Handles This For You ;-)

@connect(
  state => state.workout,
  actionCreators
)
export class WorkoutTable extends Component {
  onAddPlan = () =>{
    const {currentPlans} =  this.props
    this.props.createPlan();
  }

Cheers!

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

You can also do this to inject actions as this.props.actions:

@connect(
  state => state.workout,
  dispatch => ({
    actions: bindActionCreators(actionCreators, dispatch)
  })
)
export class WorkoutTable extends Component {
  onAddPlan = () =>{
    const {currentPlans} =  this.props
    this.props.actions.createPlan();
  }

@andre0799
Copy link
Author

Thank you very much @gaearon this explains a lot. And thanks for the awesome work you've been doing with Redux 😉

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2015

Redux, not Reflux. 😉

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