guillemcordoba / lit-svelte-stores

Lit integration to use svelte stores as cross element state management
MIT License
12 stars 2 forks source link

Additions to supply `changedProperties` in `ReactiveElement` update lifecycle #2

Open pospi opened 1 year ago

pospi commented 1 year ago

This achieves two main things:

  1. host.requestUpdate is now provided with data of the change. The key in the provided changedProperties is 'value' by default; similarly to the pattern used in other common ReactiveController implementations. In this mode the inner value attribute of the StoreSubscriber will be sent directly as the value.
  2. The key set in changedProperties can be overridden with notifyPropertyName, which allows multiple StoreSubscribers to be bound to the same ReactiveElement and differentiated in updated etc lifecycle hooks. In this mode the inner value attribute of the StoreSubscriber will be sent as a value sub-property of the returned (otherwise empty) object. In this way, changedProperties.get(notifyPropertyName).value can be compared directly with this[notifyPropertyName].value in update lifecycle equality checks.

To finalise I thought you might want to start combining these trailing parameters into an options object?

guillemcordoba commented 1 year ago

Hum... I see what you are doing...

I'm a bit wary of adding lit specific bindings to the controller. As you can see here https://github.com/lit/lit/issues/2148#issuecomment-919380347 one of the intentions behind ReactiveControllers is that they work across frameworks, and not all frameworks have the changedproperties object or concepts. In this case this is really relevant because the modules that we export around the holochain community have stores that need to work across browsers. So things like this have to be possible https://stackblitz.com/edit/vitejs-vite-tabmaq?file=src%2Fcomponents%2FMyNickname.vue.

Is the goal to have some code executed after some subscribed value gets updated? I have found myself needing this in the past as well, and there may be a better way to achieve that, for example supplying an additional function to the constructor that gets executed when that value changes.

What do you think?

pospi commented 1 year ago

If it's just changedProperties you're concerned about I think that's why their own ReactiveControllers check for update.length (which gives the parameter count on a Function) when broadcasting any events, for implementations that don't parameterise the update lifecycle hook?