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

Typing breaks when using non-string 'id' state property #253

Closed
LadislavBohm opened this issue Oct 7, 2020 · 3 comments · Fixed by #254
Closed

Typing breaks when using non-string 'id' state property #253

LadislavBohm opened this issue Oct 7, 2020 · 3 comments · Fixed by #254
Labels
bug Something isn't working discussion

Comments

@LadislavBohm
Copy link

Reproduction

export const useMainStore = defineStore({
  id: "main",
  state: () => ({
    id: 1,
    counter: 0,
  }),
  getters: {
    doubleCount() {
      //this works
      return this.counter * 2;
    },
  },
  actions: {
    reset() {
      //this no longer works, 'this' is not typed to store instance
      this.counter = 0;
    }
  }
});

Steps to reproduce the behavior

  1. Define store with state property 'id' and some type other than string
  2. Define action and try to access the store
  3. 'this' is not typed to instance of store, so cannot access state or patch it

Expected behavior

I should be able to define any properties with any types in the state.

Additional information

Most likely it's a collision with 'id' property on store itself which is typed as string. It works as expected in getters just actions are broken.

@posva
Copy link
Member

posva commented Oct 7, 2020

This is an interesting one. It makes me realize maybe all properties of the store should be prefixed with $ (maybe with the exception of patch) since they are directly accessible on the store instance. Defining an id on state will override the store's id property and break things like devtools. Same with properties named patch, subscribe, state or reset

@posva posva added bug Something isn't working discussion labels Oct 7, 2020
@LadislavBohm
Copy link
Author

Very fast response, thanks. I agree that prefixing sounds like a good idea. Maybe also prefixing _r private property to maintain consistency?

@posva
Copy link
Member

posva commented Oct 7, 2020

it's already prefixed with _ and it's unlikely to be defined by the user given its short, prefixed, and non-descriptive name, so I prefer keeping it like that. Pretty much like we have in other parts of the Vue ecosystem

@posva posva closed this as completed in #254 Oct 9, 2020
posva added a commit that referenced this issue Feb 9, 2021
Close #253. This also aligns with the current v2 API and makes migration
easier from Vue 2 to Vue 3.

BREAKING CHANGES: all store properties (`id`, `state`, `patch`, `subscribe`, and `reset`) are now prefixed with `$` to allow properties defined with the same type and avoid types breaking. Tip: you can refactor your whole codebase with F2 (or right-click + Rename) on each of the store's properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants