HenrikJoreteg / redux-bundler

Compose a Redux store out of smaller bundles of functionality.
https://reduxbundler.com
583 stars 46 forks source link

Question: What is the use case for subscribeToSelectors only passing relevantChanges rather than all subscribed keys? #66

Closed SahidMiller closed 4 years ago

SahidMiller commented 4 years ago

Hello!

I'm curious what's the reason for only sending changed values to subscribers?

I can't think of any performance issues or code smells by introducing these keys (after diffing for changes, still)... And although it's a pretty simple change to include properties from watchedValues, I'm a bit hesitant considering it's not default functionality.

Relevant part of code:

    subscriptions.forEach(function (subscription) {
      var relevantChanges = {};
      var hasChanged = false;

      if (subscription.names === 'all') {
        Object.assign(relevantChanges, changed);
        hasChanged = !!Object.keys(relevantChanges).length;
      } else {
        subscription.names.forEach(function (name) {

          if (changed.hasOwnProperty(name)) {
            relevantChanges[name] = changed[name];
            hasChanged = true;
          }
        });
      }

      if (hasChanged) {
        subscription.fn(relevantChanges);
      }
    });
  });
HenrikJoreteg commented 4 years ago

Hi @SahidMiller :wave:. I guess it just seemed most logical. What scenario do you have in which you wish it to return all?

HenrikJoreteg commented 4 years ago

Also it's harder to go the other way. If it were to return everything it subscribed to then if you only wanted the ones that actually changed you would have to reimplement diffing. /me shrugs

SahidMiller commented 4 years ago

Thanks for the honest answer! I was also working off what I thought was most logical, haha. So it was surprising to me that there wasn't any mention in the code or docs on it. (Also, since I did this same change in my last project with redux-bundler too)

I figured it's nice to have a subscriber work like a reactor, in a sense. Which also pairs nicely when used in a library with observables or promises. And I figured it's easier for a component to diff using their local ref, instead.

It might be a tomato, tomato, thing. And if it isn't, that's why I asked!