DerivedProperty should assert that its dependencies are defined

[samreid]

In Slack, @pixelzoom said (paraphrased): when I accidentally pass undefined for more than 1 dependency, DerivedProperty tells me there are duplicate dependencies instead of telling me that a dependency is undefined.

[samreid]

Proposed fix committed, I tested by changing DerivedPropertyTests:

const c = new DerivedProperty( [ a, b ], ( ( a, b ) => {return a + b;} ) );

to read:

const c = new DerivedProperty( [ undefined, b ], ( ( a, b ) => {return a + b;} ) );

And confirmed that the unit test failed with:

Assertion failed: dependencies should all be truthy

@pixelzoom do you recommend any other changes or work for this issue?

[pixelzoom]

Imo, there's a better way. This (untested) seems to have multiple advantages:

assert && assert( _.every( dependencies, dependency => !!dependency ), 'dependencies should all be truthy' )

And the current implementation:

assert && assert( dependencies.filter( d => !!d ).length === dependencies.length, 'dependencies should all be truthy' );

... violates this CRC item:

• [ ] Names (types, variables, properties, Properties, functions,...) should be sufficiently descriptive and specific, and should avoid non-standard abbreviations.

[pixelzoom]

This same assertion should also be added to Multilink. It currently has only this assertion, duplicated from DerivedProperty:

    assert && assert( dependencies.length === _.uniq( dependencies ).length, 'duplicate dependencies' );

[samreid]

I simplified the truthy check by using the every which is built-in to Array. Since its callback only requires a truthy value, we can use the identity function (no need to !!). I added this to Multilink as well.

[pixelzoom]

👍🏻 closing