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

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

Open
shasderias opened this issue Nov 19, 2019 · 6 comments

Comments

@shasderias
Copy link

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()
@davestewart
Copy link
Owner

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?

@shasderias
Copy link
Author

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.

@davestewart
Copy link
Owner

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

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

@bkarlson
Copy link

bkarlson commented May 29, 2020

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.

@davestewart
Copy link
Owner

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?

@bkarlson
Copy link

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.

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