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

Passing an effect's action to its send() creates infinite loop #114

Closed
timwis opened this issue Jul 2, 2016 · 6 comments
Closed

Passing an effect's action to its send() creates infinite loop #114

timwis opened this issue Jul 2, 2016 · 6 comments
Milestone

Comments

@timwis
Copy link
Member

timwis commented Jul 2, 2016

The following creates an infinite loop of the add effect being called over and over.

app.model({
  state: { items: [] }
  reducers: {
    receive: (action, state) => ({ items: [...state.items, action] })
  }
  effects: {
    add: (action, state, send) => {
      db.put(action).then(() => send('receive', action))
    }
  }
})

const view = html`
  <form onsubmit=${(e) => {
    send('add', { title: e.target.children[0].value })
    e.preventDefault()
  }}>
    <input>
  </form>`

After some debugging, it looks like it's because send-action puts a type property into the action parameter. And if you pass that action along, choo does not overwrite that property.

Per what I was just saying in IRC, I wonder if the idea of putting the type property in the action (like redux does) parameter adds any value since it's already the method name? It seems unnecessary to have to pick apart the object properties you want, or put them into a payload property. Going a step further, I wonder if the action parameter should be renamed something like payload? (Which would make the above code more readable)

@yoshuawuyts
Copy link
Member

Yeah, agree with you 100% here - the .type property should be omitted; something that's taken care of in v3. Going to mark this a bug, breaking change and add it to the v3 milestone since it's already fixed. Thanks! ✨

@yoshuawuyts yoshuawuyts added this to the 3.0.0 milestone Jul 2, 2016
@yoshuawuyts yoshuawuyts mentioned this issue Jul 2, 2016
13 tasks
@yoshuawuyts
Copy link
Member

Closing as 3.0 is imminent; wanna make sure all issues are taken care of. Releasing soooon™ ✨

@crapthings
Copy link

crapthings commented Jul 13, 2016

it causes infinite loop when i put onload

onload=${send('fetch')}

model.js

const faker = require('faker')

const times = require('lodash').times

app.model({

  state: {
    users: []
  },

  effects: {
    fetch: (data, state, send, done) => {
      let users = times(10, n => {
        return {
          username: faker.internet.userName()
        }
      })

      send('init', { payload: users }, done)
    }
  },

  reducers: {
    init: (data, state) => {
      return {
        users: data.payload
      }
    },

    add: (data, state) => {
      return {
        users: state.users.concat(data.payload)
      }
    }
  }

})

@timwis
Copy link
Member Author

timwis commented Jul 14, 2016

Hi @crapthings, that's a different issue than what's described above, though I can see how it would cause an infinite loop. What you mean to write is onload=${(el) => send('fetch')}, which is a function declaration. What you wrote, onload=${send('fetch')} is executed every time the view is run and onload is "setup" rather than when onload is actually triggered.

@crapthings
Copy link

should i use onload to get inital data ?

i've found onload only happend on page reload, not switch route

@yoshuawuyts
Copy link
Member

i've found onload only happend on page reload, not switch route

It should tho, related to #1 perhaps?

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

3 participants