vuex-pathify icon indicating copy to clipboard operation
vuex-pathify copied to clipboard

should the get() component helper reference getters, vis-a-vis variable expansion

Open shasderias opened this issue 5 years ago • 6 comments

Thank you for the library! Ran into a "problem" when using the get() component helper. Not sure if a change is necessary (other than perhaps clarifying the documentation). For your consideration.

When used as a component helper, it seems that:

  1. get() references getters and state; and
  2. sync() references state only.

I think in line with Pathify's accessor priority, this is fine. But when the path passed to get() references a getter and there is a variable to be expanded in the path, get() does not work. See the following link for a repro:

Code Sandbox

There are a few distinct issues that might be worth addressing here:

  • conceptually, should get() (when used as a component helper) reference getters?
  • if the above is true, how should get() deal with variable expansion when referencing getters?
  • if variable expansion is not meant to work when referencing a getter, perhaps Pathify should return an error, currently, get() fails silently and always returns undefined
  • if nothing else, the documentation for variable expansion could be clearer in this regard (https://davestewart.github.io/vuex-pathify/#/api/paths)
  • the documentation states "when getting, only state will be referenced; getters will be ignored", right below an example of get(), when this is only true for sync()

shasderias avatar Nov 19 '19 04:11 shasderias

Hey @shasderias

Thanks for the ticket and demo.

You mention variable expansion, but I can't see that in the demo link you posted.

And I can't tell what problem you are having from the demo, as it seems to work as intended.

Is that correct?

davestewart avatar Nov 19 '19 22:11 davestewart

Ugh. Apparently I can't work websites.

Code Sandbox

Relevant statements from App.vue, lines 52-54 (1) aryByGet: get("ary@[:index]"), (2) aryBySync: sync("ary@[:index]"), (3) aryByGetNoSubPropOperator: get("ary[:index]")

Just discovered while recreating the demo, that in addition to (2), (3) also works. The issue I was trying to report is why (2) works, but (1) doesn't. I wonder if this is user error.

Sorry for the noise, please take another look.

shasderias avatar Nov 19 '19 22:11 shasderias

That's OK, I don't mind helping!

Maybe you're the lucky one who's found a bug 😆

davestewart avatar Nov 19 '19 23:11 davestewart

Just stumbled upon this as well, for those who come here googling:

This works:

  ...get('userStore/', {cart: 'carts[:lastShopId]'}),
  ...sync('userStore/', {syncCart: 'carts@:lastShopId'}),

While this does not:

...get('userStore/', {cart: 'carts@:lastShopId'}),

From reading the docs I cannot graps why second get is not defined.. but at least there's a workaround.

bkarlson avatar May 29 '20 15:05 bkarlson

OK, there's been quite a lot of activity here.

I guess thing for me to look at is this:

...when a path passed to get() references a getter and there is a variable to be expanded in the path, get() does not work

Is that right?

davestewart avatar May 29 '20 17:05 davestewart

yes, correct, but the getter is created by pathify itself, i.e. I'm just trying to access a property in a storage using ...get() with var expansion.

bkarlson avatar May 31 '20 13:05 bkarlson