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

Bind dispatch on events #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LeMisterV
Copy link

Hello Edvin,

Que pense tu de cabler le dispatch sur les events d'un component ?
Avec ce patch on a les deux: binding sur les events, et injection des actions en data... chacun choisi sa religion ;)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 07fa564 on LeMisterV:dispatch-events into c1bfd0f on edvincandon:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 93e6f2e on LeMisterV:dispatch-events into c1bfd0f on edvincandon:master.

@edvincandon
Copy link
Owner

edvincandon commented Dec 8, 2017

Hey thx :) Le use-case ce serait pour catcher au passage l'event émis dans un composant parent?

Du coup je pense qu'on pourrait simplifier et passer par une option dans la fonction connect()

Tu penses quoi de faire ça :
Ne faire le bindActionCreators qu'une seule fois :

let actionCreators
return { 
// ...
data () {
    actionCreators = bindActionCreators(mapDispatch, this.$$store.dispatch)
// ...
}

Du coup en gros on pourrait mapper directement le mapDispatch sur les events en passant l'option
{ mapToEvents: true } - je vois pas trop de raison pour séparer les actionCreators dans deux objects distincts.

et en suite dans le created() :

if (options.mapToEvents) {
    Object.keys(actionCreators).forEach(key => vm.$on(key, actionCreators[key])) 
    // plus besoin du forEach custom
}

Du coup le connect final sur un composant ressemblerait à ça:
connect(mapState, mapDispatch, { mapToEvents: true })

Bout à bout:
connector.js :

const connector = (_mapState = defaultMapState, mapDispatch = defaultMapDispatch, options = { mapToEvents: false }) => component => {
  const mapState = normalizeMapState(_mapState)
  let actionCreators

  return {
    name: `connect-${component.name}`,
    mixins: [component],
    inject: ['$$store'],

    data () {
      actionCreators = bindActionCreators(mapDispatch, this.$$store.dispatch)

      const initData = {}
      const mapData = {
        ...mapState(this.$$store.getState()),
        ...actionCreators
      }

      Object.keys(mapData).forEach(key => {
        initData[key] = mapData[key]
      })

      return initData
    },

    created () {
      const vm = this

      if (options.mapToEvents) {
        Object.keys(actionCreators).forEach(key => vm.$on(key, actionCreators[key]))
      }

      const getMappedState = state => mapState(state)

      const observeStore = (store, select, onChange) => {
        let currentState = select(store.getState())

        function handleChange() {
          const nextState = select(store.getState())
          if (!shallowEqual(currentState, nextState)) {
            const previousState = currentState
            currentState = nextState
            onChange(currentState, previousState)
          }
        }

        return store.subscribe(handleChange)
      }

      this._unsubscribe = observeStore(this.$$store, getMappedState, (newState, oldState) => {
        Object.keys(newState).forEach(key => {
          vm.$set(this, key, newState[key])
        })
      })
    },

    beforeDestroy() {
      this._unsubscribe()
    }
  }
}

@edvincandon
Copy link
Owner

Also: je viens de modifier quelques outils de dev sur master

@LeMisterV
Copy link
Author

Je use-case, c'est plutôt de binder une action redux soit dans un data comme c'était fait avant, soit sur un event émis par le component.
Suivant le comportement du composant, ça peut rendre le code plus simple/claire/cohérent de cabler les actions redux sur les events mais ça peut laisser plus de liberté de pouvoir cabler sur des data.
Donc avoir les deux solutions me semble intéressant.

Mais ça ne me semble pas pertinent d'avoir les deux à la fois. systématiquement.

@edvincandon
Copy link
Owner

edvincandon commented Dec 9, 2017

Je vois - t'aurais un exemple de use-case ou le fait de mapper les actions sur les events du composant aurait une autre utilité que l'économie de faire manuellement des:
this.$on('action', this.doAction) dans le created()this.doAction est un action creator?

@LeMisterV
Copy link
Author

LeMisterV commented Dec 9, 2017 via email

@edvincandon
Copy link
Owner

@LeMisterV ping pour avoir des news ;)

@LeMisterV
Copy link
Author

LeMisterV commented Dec 17, 2017 via email

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

Successfully merging this pull request may close these issues.

None yet

3 participants