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

Defining store computed property after it is referenced in a template causes initial value to be undefined #1327

Closed
btakita opened this issue Apr 8, 2018 · 4 comments · Fixed by #1395
Labels

Comments

@btakita
Copy link
Contributor

btakita commented Apr 8, 2018

It does not seem like store can be used in the repl (sveltejs/v2.svelte.dev#213), so I'll explain the issue here.

I am mixing in computed properties into a store. It does not seem like the initial value of newly created store computed properties bubble up to the template.

<p>{{$name}}</p>

<script>
  export default {
    oncreate() {
      this.store.compute('name', ['user'], user => user && user.name)
      this.store.set({user: {name: 'Joe'}})
    }
  }
</script>

To alleviate this issue, I added a compute wrapper:

export function compute(store, name, deps, fn) {
  const values__deps = []
  for (let i=0; i < deps.length; i++) {
    values__deps.push(store.get(deps[i]))
  }
  const __set = {}
  __set[name] = fn(...values__deps)
  store.set(__set)
  store.compute(name, deps, fn)
  return store
}
btakita added a commit to btakita/embessenger that referenced this issue Apr 8, 2018
---

__/channels/Groups.html

  + group__selected
    - selected_group

__/channels/store.mjs

  + __selected__channels
    - __channels
    * store
      + channel_id__selected__channels
        - selected_channel_id
      + messages__selected__channels
        - messages__selected_channel
      + channel__selected__channels
        - selected_channel
      + push__messages__selected__channels
        - push__messages__selected_channel

__/conversation/Message.html

__/conversation/store.mjs

  * __conversation
    * store
      + group__selected
        - selected_group

__/groups/store.mjs

  * __groups
    * store
      + title__group__selected__groups
        - selected_group_title

__/layout/Overlay.html

__/store/lib.mjs

  + $store
    ⊂ window.store
  + compute
    * before defining computed property
      * set initial value
      * Fixed glitch with initial rendered value
        * see sveltejs/svelte#1327

package.json

  ⊃ "sapper": "^0.10.5"
  ⊃ "svelte": "^1.60.2"
  ⊃ "svelte-loader": "^2.8.1"
  ⊃ "webpack": "^4.5.0"
  ⊃ "webpack-cli": "^2.0.14"

routes/4xx.html

routes/5xx.html

routes/about.html

routes/blog/[slug].html

routes/blog/index.html

routes/index.html

webpack/client.config.js

webpack/server.config.js
@btakita
Copy link
Contributor Author

btakita commented Apr 23, 2018

With the v2 upgrade, the compute helper function is now:

export function compute(store, name, deps, fn) {
  const values__deps = []
  const state = store.get()
  for (let i=0; i < deps.length; i++) {
    values__deps.push(state[deps[i]])
  }
  const __set = {}
  __set[name] = fn(...values__deps)
  store.set(__set)
  store.compute(name, deps, fn)
  return store
}

@Rich-Harris
Copy link
Member

Thanks — it looks like this works in the current version — it's possible that it was fixed since you opened this.

Let me know if I'm closing this issue in error.

@btakita
Copy link
Contributor Author

btakita commented Apr 30, 2018

It's a matter of the ordering. The issue occurs if you call this.store.set before this.store.compute...

https://svelte.technology/repl?version=2.2.0&gist=a0548b9dafee301e890541a0d1ee4c6a

@Rich-Harris Rich-Harris reopened this Apr 30, 2018
Rich-Harris added a commit that referenced this issue May 1, 2018
@arxpoetica arxpoetica added the bug label May 1, 2018
Rich-Harris added a commit that referenced this issue May 3, 2018
Rich-Harris added a commit that referenced this issue May 3, 2018
Update store state when new computed properties are added
@Rich-Harris
Copy link
Member

Fixed in 2.4.3, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants