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

Updating property on an object in a store triggers reactivity for the whole store #4285

Closed
frederikhors opened this issue Jan 19, 2020 · 12 comments

Comments

@frederikhors
Copy link

frederikhors commented Jan 19, 2020

DISCLAIMER: I'm still learning Svelte. Maybe I'm wrong in what I say.

I'm searching a good pattern to start with Svelte's stores.

I thought I had found a decent one but I was wrong.

As you can see from the REPL here: https://svelte.dev/repl/8e628e5b3c8d4bddb9126fc82e090674?version=3.17.1, when I update a single property of the object (state) in the store, all the other properties are affected.

Is there a way to avoid this and trigger reactivity just for that property around in my components?

This comment: #3053 (comment) from @nodefish gave me thoughts.

Some code

  • App.svelte:
<script>
  import playerStore from './playerStore.js'
	
  playerStore.init()
	
  $: isBoy = ($playerStore.isBoy || !$playerStore.isBoy) && Date.now()
  $: isGirl = ($playerStore.isGirl || !$playerStore.isGirl) && Date.now()
  $: isSick = ($playerStore.isSick || !$playerStore.isSick) && Date.now()
</script>

{JSON.stringify($playerStore)}
<br />
<br />
isBoy: {Date.now()} - {isBoy}
<br />
<br />
isGirl: {Date.now()} - {isGirl}
<br />
<br />
isSick: {Date.now()} - {isSick}
<br />
<br />

<button on:click={() => playerStore.toggleIsSick()}>Toggle isSick</button>
  • playerStore.js:
import { writable } from 'svelte/store'

const store = () => {
  console.log('*: playerStore -> store()')
	
  const state = {
    isBoy: undefined,
    isGirl: undefined,
    isSick: undefined
  }
	
  const { subscribe, set, update } = writable(state)
	
  const methods = {
    init () {
      console.log('*: playerStore -> init()')
      update(state => {
        state.isBoy = false
        state.isGirl = false
        state.isSick = false
      return state
      })
    },
		
      setIsBoy () {
      console.log('*: playerStore -> setIsBoy()')
	update(state => {
          state.isBoy = true
          state.isGirl = false
          return state
      })
     },
		
      setIsGirl () {
      console.log('*: playerStore -> setIsGirl()')
	update(state => {
        state.isBoy = false
        state.isGirl = true
        return state
      })
	},
		
	toggleIsSick () {
      console.log('*: playerStore -> toggleIsSick()')
	update(state => {
        state.isSick = !state.isSick
        return state
      })
	}
}

  return {
    subscribe,
    set,
    update,
    ...methods
  }
}

export default store()
@pngwn
Copy link
Member

pngwn commented Jan 19, 2020

The whole store does indeed react to changes. That is because reactivity is on the level of the store itself rather than the values within. If this is an issue the you will need to design your custom stores to deal with this or use smaller stores.

However if your UI only relies on specific store values then the DOM will only be modified when those values actually change. In most cases the fact that the whole store is updated doesn’t matter too much and the expensive parts, DOM manipulation, are only performed when the actual values themselves change.

@pngwn
Copy link
Member

pngwn commented Jan 19, 2020

I’m curious. What problem is this actually creating?

@kevmodrome
Copy link
Contributor

However if your UI only relies on specific store values then the DOM will only be modified when those values actually change. In most cases the fact that the whole store is updated doesn’t matter too much and the expensive parts, DOM manipulation, are only performed when the actual values themselves change.

Maybe this (or some variation of it) could be added to the Store API documentation? I suspect this is not only relevant to stores though.

@frederikhors
Copy link
Author

The whole store does indeed react to changes. That is because reactivity is on the level of the store itself rather than the values within. If this is an issue the you will need to design your custom stores to deal with this or use smaller stores.

I'm looking for a pattern/best practices indeed. Do you like this way? Can you suggest another?

However if your UI only relies on specific store values then the DOM will only be modified when those values actually change. In most cases the fact that the whole store is updated doesn’t matter too much and the expensive parts, DOM manipulation, are only performed when the actual values themselves change.

How much work is done just comparing values? Why it's not "expensive"?

@frederikhors
Copy link
Author

However if your UI only relies on specific store values then the DOM will only be modified when those values actually change. In most cases the fact that the whole store is updated doesn’t matter too much and the expensive parts, DOM manipulation, are only performed when the actual values themselves change.

Maybe this (or some variation of it) could be added to the Store API documentation? I suspect this is not only relevant to stores though.

What do you mean?

@kevmodrome
Copy link
Contributor

What do you mean?

I mean that if this was described in the documentation there would be a greater understanding of how Svelte works.

Generally I think that the whole store reacting to changes is not a problem since it's not an expensive operation. The size of your store would need to be massive for it to affect performance.

@pngwn
Copy link
Member

pngwn commented Jan 19, 2020

Firstly, Svelte's main job is to generate optimal code paths for updating the DOM and only perform those updates when they are necessary. It doesn't really matter what happens before that, the process by which svelte updates is pretty much the same (whether you use a store or local component state):

<h1>{ content }</h1>
<button on:click={ () => content = 'new value' }>Change</button>

The click handler will generate code that is conceptually the same as the following (it doesn't actually generate this code but this is the process that updates follow):

content = 'new value';
if (content !== current_value_used_in_the_h1) update_h1(content);

So if you assign the same value to content the h1 won't update because Svelte knows that it is still the same.

This is still true if you use a store with nested properties:

<script>
  import { writable } from 'svelte/store';

  const store = writable({ x: 1, y: 2, z: 3 });
</script>

<h1>{$store.x }</h1>
<button on:click={ () => $store.x = 4 }>Change</button>

What happens in the store doesn't matter much when it comes to updating the actual DOM because Svelte is interested in the actual value that is being used in the h1. It isn't comparing the whole store with the store's previous value, it will simply check to see if whatever is used in the h1 ($store.x) is the same value that it was previously. In this case if $store.x is still 1 then it will do nothing, if it has changed then it will update. The structure of the store itself doesn't matter. In general, DOM updates are by far the most expensive thing you will do in a browser after everything has loaded, the structure of your data doesn't matter too much, you will still get optimal updates. These comparisons are very cheap, you would have to perform a huge number of them at the same time to run into problems.

When the structure of your store and the frequency of updates do matter, is when you are doing expensive work inside them or triggering side-effects when they update. For example, if you are performing complex data transforms when values change or triggering various function calls when a store property updates. If you only actually care about certain properties of the store then you either need to guard against these kinds of unnecessary updates in the store itself or split the single structure you have into separate parallel structures. This gives you more control over when you trigger actions in response to store updates.

There are other options available to you, such as deconstructing store properties and reacting only to updates of those deconstructed values rather than to the store itself (you can see an example in this REPL - check the console as you click).

tldr: when it comes to DOM updates: don't worry, it won't make a difference. If you are performing expensive work or side-effects: split your stores or write something custom to guard against undesirable behaviour. State management is your business though, and it is best to treat a single store as a single reactive unit: expect the whole thing to update when any part of it updates and proceed from there. Most of the time this won't matter.

Svelte will handle the DOM stuff, don't worry about it!

@pngwn
Copy link
Member

pngwn commented Jan 19, 2020

Regarding good store patterns: it really depends on a lot of other factors as well. In general, I try to keep stores small and focused, compose more complex stores out of focused parallel structures (by deriving them), and be very careful. when executing side-effects in response to store changes. Ideally, effectful code should be isolated as much as possible. Other than that store design is more likely to be dictated by the rest of your architecture than anything else.

@kevmodrome Yeah, ee probably could add something simple about not needing to worry about this in the docs. It might put some folks mind at ease.

@frederikhors
Copy link
Author

There are other options available to you, such as deconstructing store properties and reacting only to updates of those deconstructed values rather than to the store itself (you can see an example in this REPL - check the console as you click).

Thanks for your answer.

Can I ask you why that example in REPL?

What changes between $s.x and the deconstructed x?

@vipero07
Copy link

Depending on your intent, you may wish to split the store up like https://svelte.dev/repl/a9d85c512f66477fba10699e3ae361b2?version=3.17.1

However, as @pngwn points out your example would work better as https://svelte.dev/repl/6dfa9e69a65243b0bbc44d66d91b8325?version=3.17.1

Personally I would dump the init method and just use it for state instead of setting state = undefined above.

like https://svelte.dev/repl/d14edf8fbad840a9a0e0314f14fd9ba9?version=3.17.1

I know this is only a test example, but isGirl or isBoy isn't necessary since its just inverting the other and would be easy enough to test if necessary on update (like using a derived store), as opposed to storing the bit in memory for both.

@frederikhors
Copy link
Author

frederikhors commented Jan 25, 2020

Thank you all for your answers.

@vipero07 I saw your playerStore here: https://svelte.dev/repl/d14edf8fbad840a9a0e0314f14fd9ba9?version=3.17.1.

You used this:

setIsBoy () {
  console.log('*: playerStore -> setIsBoy()')
  update(state =>  ({...state, isBoy: true, isGirl: false}))
}

instead of this:

setIsBoy () {
  console.log('*: playerStore -> setIsBoy()')
  update(state => {
    state.isBoy = true
    state.isGirl = false
    return state
  })
}

If I use your way (update(state => ({...state, isBoy: true, isGirl: false}))) I cannot use the code below, right?

toggleCoach () {
  console.log('*: playerStore -> toggleCoach()')
  if (state.isBoy) {
    console.log("isBoy")
    // ...
  } else {
    console.log("isGirl")
    // ...
  }
}

If I use your code I cannot read anymore the up-to-date state object, right? I need to use Svelte's get() or check it in update() method, right?

Is my method wrong even if more verbose?

@vipero07
Copy link

vipero07 commented Jan 27, 2020

@frederikhors more verbose is fine, the way I wrote is is the equivalence of

setIsBoy () {
  console.log('*: playerStore -> setIsBoy()');
  update(state =>  { 
    // return Object.assign({}, state, {isBoy: true, isGirl: false}); also works here
    return {...state, isBoy: true, isGirl: false};
  });
}

The biggest difference (and this may not matter to you) is setting .isBoy and .isGirl on the original state mutates the original object. Meaning minimally, you can no longer access the original state. Personally I try to avoid object mutation and assume everything is immutable. Here it probably doesn't make a difference, but I think of it like this:

setIsBoy () {
  console.log('*: playerStore -> setIsBoy()');
  update(state =>  { 
    // copy state to new object
    var newState = Object.assign({}, state);
    newState.isBoy = true;
    newState.isGirl = false;
    // can still check old state here
    console.log(`State isBoy changed from ${state.isBoy} to ${newState.isBoy}`);
    return newState;
  });
}

More importantly however readability is important. So keep it with what you are familiar with.

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

5 participants