[Discussion] Remove changed checks for simple computed properties

[jacobp100]

I have the input,

monthlyAdminCosts: (numDrivers, driverMileageTime, driverSalary, adminMileageTime, adminSalary) =>
  numDrivers * driverMileageTime * driverSalary + adminMileageTime * adminSalary,

Which gives the large output,

if ( isInitial || ( 'numDrivers' in newState && typeof state.numDrivers === 'object' || state.numDrivers !== oldState.numDrivers ) || ( 'driverMileageTime' in newState && typeof state.driverMileageTime === 'object' || state.driverMileageTime !== oldState.driverMileageTime ) || ( 'driverSalary' in newState && typeof state.driverSalary === 'object' || state.driverSalary !== oldState.driverSalary ) || ( 'adminMileageTime' in newState && typeof state.adminMileageTime === 'object' || state.adminMileageTime !== oldState.adminMileageTime ) || ( 'adminSalary' in newState && typeof state.adminSalary === 'object' || state.adminSalary !== oldState.adminSalary ) ) {
  state.monthlyAdminCosts = newState.monthlyAdminCosts = template.computed.monthlyAdminCosts( state.numDrivers, state.driverMileageTime, state.driverSalary, state.adminMileageTime, state.adminSalary );
}

Would it make sense for simple computed properties to not do these checks? We could define a simple computed property as one that does not invoke any functions.

As a quick test, I removed these checks from a simple project, and it decreased the min+gz size by 5%.

[Rich-Harris]

It's an interesting thought and one definitely worth exploring. Part of the problem is that while the computation itself might be trivial, something less trivial could depend on that computation:

<ComplexComponent selection='{{selection}}'/>

<script>
  export default {
    data: function () {
      return {
        items: [...],
        selectedIndex: 42
      };
    },

    computed: {
      selected: function ( items, selectedIndex ) {
        return items[ selectedIndex ];
      }
    }
  };
</script>

Now, say you did component.set({ selectedIndex: 42 }). Right now, that has basically no effect — the selected value isn't recomputed because selectedIndex hasn't changed. And because selected isn't part of the object representing changed state, <ComplexComponent/> is left alone.

Without the check, selected would be recomputed and added to the changed state object. Which means that we would have to update <ComplexComponent/>. That might have nested components and computations of its own, so avoiding that initial check ends up having far-reaching implications.

FWIW, that code will get a little less verbose as of #413, which I plan to tackle this morning.

[PaulBGD]

We could move it into a helper function, but due to monomorphism it would end up being slower. I don't think there's an easy way to make the code smaller here, because it does need to check each of the properties for performance.

[Rich-Harris]

@PaulBGD what about the differs helper introduced in #416, is that a good solution?

[PaulBGD]

@Rich-Harris It'll only be as fast as the current solution if the passed parameters to differs are of the same type (ex boolean, string, number) that were used last time it was called, so it probably will be slightly slower unless it's only comparing strings or something. The helper is a good solution for cutting down code however.

[Rich-Harris]

Hmm. Would this be a good intermediate solution?

function isMutable ( thing ) {
  var type = typeof thing;
  return thing && type === 'object' || type === 'function';
}

if ( isInitial || ( 'foo' in newState && state.foo !== oldState.foo || isMutable( state.foo ) ) ) {
  state.bar = newState.bar = template.computed.bar( state.foo );
}

How much potential impact are we talking about?

One thing I've idly wondered about is whether we could leverage type information to remove some of the ambiguity in cases like these. For example if we knew that foo was an object, array or function that example above could become this:

if ( isInitial || 'foo' in newState ) {
  state.bar = newState.bar = template.computed.bar( state.foo );
}

In the case where we encounter {{foo.bar}} or {{#each foo...}} we can very easily determine the type. Bit trickier in the case of <Child :foo/>, unless we analyse the computation itself (which quickly becomes quite rocky terrain), but maybe we do it on a best-effort basis (or leverage something like Flow internally?) and allow motivated users to fill in the blanks like so:

<script>
  export default {
    data () {
      return { foo: couldBeAnything };
    },

    computed: {
      bar: foo => doSomethingWith( foo )
    },

    types: {
      foo: Object
    }
  };
</script>

Nice thing about that approach is you can remove the types block from the compiled output while still using it to throw errors in dev mode if the wrong kind of data is supplied.

[PaulBGD]

So from what I've found, with primitives it doesn't matter very much. There's definitely a slowdown with different objects, but passing a string one moment and a number the next it seems the performance isn't that bad (I'm assuming because the JS engine can cache a few different types before discarding.) Looking over the numbers, your differ function would be fine in terms of performance and code cutting.

[Rich-Harris]

As of recent versions, Svelte uses the differs helper which results in much neater code. We could still leverage type information to reduce it further, but I don't think we should go beyond that due to the potential for cascading effects as outlined above — will close this issue. Thanks.