davestewart / vuex-pathify

Vue / Vuex plugin providing a unified path syntax to Vuex stores
https://davestewart.github.io/vuex-pathify
MIT License
1.37k stars 57 forks source link

[Help Needed] Unknown local action type: . . , global type: . . . #47

Closed CelticParser closed 5 years ago

CelticParser commented 5 years ago

Great plug! Just started implementing it.

I'm getting a:

unknown local action type: setSatsAvail, global type: AvWxData/setSatsAvail

Not sure what I have wrong (because the other setters work fine). It be appreciated if you can make sense of what I've got here.

The AvWxData module:

import { make } from 'vuex-pathify'

import {
  fetchKpIndex,
  fetchKpForecast,
  fetchAvailSatData,
  fetchVisSatData,
} from './actions'

const state = {
  currentKp  : '',
  kpForecast : '',
  satsAvail  : '',
  satsVis    : '',
}

// make all mutations auto-magically
const mutations = make.mutations(state)

const actions = {
  ...make.actions(
    'currentKp',
    'kpForecast', // TODO: [FETCH-API]
    'satsAvail', // BROKE: Does not set by the `vuex-pathy` api @actions.fetchVisSatData 
    'satsVis',  // TODO: [FETCH-API]
  ),

  fetchKpIndex,
  fetchKpForecast,
  fetchAvailSatData,
  fetchVisSatData,
}

// make all getters auto-magically too!
const getters = {
  ...make.getters(state),
}

export default {
  namespaced: true,
  state,
  mutations,
  actions,
  getters,
}

In my actions.js:

import kpAPI        from './kpData.api'
import satelliteAPI from './satelliteData.api'

const fetchKpIndex = ({ dispatch }) => {
  kpAPI.fetchCurrentKp.then(data => dispatch('setCurrentKp', data, {
    // The Kp Index updates every 3hrs on the server so lets not hit
    // it more than needed.
    timeout: 120000, // = 2min
  }))
}

const fetchKpForecast = ({ dispatch }) => {
  kpAPI.fetchKpForecast.then(data => dispatch('setKpForecast', data))
}

const fetchAvailSatData = ({ dispatch }) => {
  satelliteAPI.fetchAvailSatData.then(data => dispatch('setSatsAvail', data))
}

const fetchVisSatData = ({ dispatch }) => {
  satelliteAPI.fetchVisSatData.then(data => dispatch('setSatsVis', data)) // <== ERRORS HERE!
}

export {
  fetchKpIndex,
  fetchKpForecast,
  fetchAvailSatData,
  fetchVisSatData,
}

Within the satelliteData.api.js:

const fetchAvailSatData = new Promise((resolve) => {
  fetch(satAvailURL)
    .then(status)
    .then(json)
    .then(function(data) {
      const satAvail = data.info.satcount
      console.log('Visible sats: ' + satAvail) // <== Prints back-loud-and-clear!
      resolve(+satAvail)
    })
})

In some parent Vue:

// . . .
beforeCreate () {
    this.$vuexGeolocation.getCurrentPosition()
    // register a new module only if doesn't exist
    if (!(this.$store.state[AvWxData])) {
      this.$store.registerModule('AvWxData', AvWxData);
    }
  }, // . . .

...and in a component:

mounted () {
  this.$store.dispatch('AvWxData/fetchAvailSatData')
  this.$store.dispatch('AvWxData/fetchKpIndex')
},
CelticParser commented 5 years ago

Hello?

davestewart commented 5 years ago

Sorry Jason - I've been pulled in too many directions in the last short while!

Totally my fault and I'm sorry.

A few questions / comments:

Just looking at your code:

const fetchAvailSatData = ({ commit }) => {
  satelliteAPI.fetchAvailSatData.then(data => commit('satsAvail', data))
}

const fetchVisSatData = ({ commit }) => {
  satelliteAPI.fetchVisSatData.then(data => commit('satsVis', data))
}

Also, it looks like fetchAvailSatData is an already-resolved promise. Is this not weird? Why not:

function fetchAvailSatData() {
  return fetch(satAvailURL)
    .then(status)
    .then(json)
    .then(function(data) {
      return data.info.satcount
    })
})

Additionally, you could just have this in your store (that's what it's there for), or log the result in the store:

const fetchAvailSatData = ({ commit }) => {
  satelliteAPI.fetchAvailSatData().then(data => {
    console.log('data received!', data)
    commit('satsAvail', data)
  })
}

Sorry to be so critical, you just seem to be writing lots and lots of extra code that is just adding to the cruft, when it should just be a simple fetch data and commit.

But happy to help you with Pathify if you think the error is there.

davestewart commented 5 years ago

OK, so I just dumped your code into my IDE for some refactoring.

It could be as simple as this:

import { make } from 'vuex-pathify'
import { fetchKpForecast, fetchCurrentKp } from './kpData.api'
import { fetchVisSatData, fetchAvailSatData } from './satelliteData.api'

const actions = {
  fetchKpIndex ({ commit }) {
    // the set-timeout stuff would be better off in the `fetchCurrentKp()` function
    fetchCurrentKp().then(data => commit('currentKp', data))
  },

  fetchKpForecast ({ commit }) {
    fetchKpForecast().then(data => commit('kpForecast', data))
  },

  fetchAvailSatData ({ commit }) {
    fetchAvailSatData().then(data => commit('satsAvail', data))
  },

  fetchVisSatData ({ commit }) {
    fetchVisSatData().then(data => commit('satsVis', data))
  },

}

const state = {
  currentKp  : '',
  kpForecast : '',
  satsAvail  : '',
  satsVis    : '',
}

// make all mutations auto-magically
const mutations = make.mutations(state)

// export
export default {
  namespaced: true,
  state,
  mutations,
  actions,
}

My feeling is, you don't really need pathify in this situation.

Just call the data as you need it:

mounted () {
  this.$store.dispatch('AvWxData/fetchAvailSatData')
  this.$store.dispatch('AvWxData/fetchKpIndex')
},

You could use pathify get if you need it, but vuex mapState will do you just as well.

Pathify is really designed for complex state mapping, sub-objects, etc.

There are times (just like Vuex!) where you don't really need it.

CelticParser commented 5 years ago

Thnx for getting back. I came back here because of issue #50 and had forgotten myself about this issue (my team doesn't use github and its been several weeks since I added this issue). Anyways, I went back into this project and now I remember; Pathify was dropped for that module.

davestewart commented 5 years ago

OK, no problems. Feel free to comment there as required.

davestewart commented 5 years ago

Also, not sure how much API work you do, but I have another package called Axios Actions that is designed to remove API calls from the store, and this case (unless the data is shared) you might not need a store at all:

https://github.com/davestewart/axios-actions/

Check it out, it might be useful to you.