goatslacker / alt

Isomorphic flux implementation
http://alt.js.org/
3.45k stars 323 forks source link

Give a warning if bindActions fails to bind? #617

Open bebraw opened 8 years ago

bebraw commented 8 years ago

Let's say you have code like this:

class LaneStore {
  constructor() {
    this.bindActions(LaneActions);
  }
  ...
}

It would be useful if Alt gave warnings for missing bindings during development. Now it's easy to typo a method and miss it given it fails silently.

goatslacker commented 8 years ago

I've thought about that but then that means that you'll have to add every method in the actions to the store.

bebraw commented 8 years ago

Is there a case where that's a good practice? Can you give a concrete example?

goatslacker commented 8 years ago

Not all actions map 1:1 to stores.

UserActions
  userAuthorized
  userUpdated

UserStore
  bindActions(UserActions)

  userUpdated(user) {
    this.setState({ user })
  }

CompanyStore
  bindActions(UserActions)

  userAuthorized() {
    this.setState({ users: this.state.users + 1 })
  }

There's also potentially the case where an action is handled in multiple stores and bindActions is lazily used, we wouldn't want to require the definition of all other actions.

bebraw commented 8 years ago

Yeah, great point. What if you could mark actions as strict somehow? Maybe there's a neat way to solve it on definition level. This will make it more verbose, but it will also communicate intent far better.

goatslacker commented 8 years ago

I'm cool with that :) .isRequired lol

bebraw commented 8 years ago

I'm cool with that :) .isRequired lol

Yeah. I would be happy with a little schema definition. Safer to use, harder to break.