Disable the state-changed event when not needed

[TimPietrusky]

Every time the state is changing, the "state-changed" event is dispatched onto every element that is using the ReduxMixin. So the more elements I have, the more events are dispatched, even when I don't want to listen to them at all.

But why is this a problem? This behaviour is increasing the scripting time at least 25% (up to 50% max), which is slowing down the overall performance. It even gets worse when using requestAnimationFrame.

Proposal

Make the "state-changed" event optional:

When creating the mixin:

export default function PolymerRedux(store, mixinProperties = { enableStateChangedEvent: true }) {

And in the redux listener:

// Redux listener
const unsubscribe = store.subscribe(() => {
    const detail = store.getState();
    update(detail);

    if (mixinProperties.enableStateChangedEvent) {
        element.dispatchEvent(new CustomEvent('state-changed', {detail}));
    }
});

Of course I could extend the mixin and make my changes, but as this is something that also might affect others I think this should be part of PolymerRedux itself.

Additional info

• I'm on the https://github.com/tur-nr/polymer-redux/tree/polymer-3 branch
• Tested on Chrome Canary 66 by using 30 components (20 of them are using the ReduxMixin)

[pixelass]

@tur-nr I can take a look at it and propose a PR on the weekend. Let's see how this looks when implemented.

[pixelass]

@TimPietrusky

• do all of the 20 components actually need the mixin?
• what is the expected behaviour when state changes are disabled?
  • I guess you expect redux to update the store without sending an event?

[tur-nr]

I think this is down to the use of the library. You should be using higher order elements that make use of the Mixin and then pass the properties down.

I don't like the idea of opting out of events, it's in the developers hands not to use the Mixin for every component that needs state. This is more beneficial for your application as it decouples your elements from Redux and makes them more portal.

[pixelass]

@tur-nr I agree. Thus the question if all 20 components really need this.

[TimPietrusky]

I have a lot of different components that do different things on the data in the state by using reselect and I need the Redux store for using selectors. That's why I add the ReduxMixin to be able to access the state.

But even if I would optimize this: My app gets new components every month and they all need their own space in the state.

I agree that changing polymer-redux might be a bit too much.

Proposal

• In my project I will extend PolymerRedux and overwrite unsubscribe to remove the state-changed event
• Contribute the example

[TimPietrusky]

@tur-nr I was wondering: Why do we even need the state-changed event?

[tur-nr]

In case a developer wants to subscribe to state changes to perform logic outside of bindings. Much like the store.subscribe method. I can't presume every element has access to the original store, so it get's proxied via this event.

[developerium]

I think the resulting flexibility is beneficial for developers.