DerivedProperty creates a general order dependency

[zepumph]

Discovered in https://github.com/phetsims/axon/issues/215 and more specifically https://github.com/phetsims/expression-exchange/issues/161, the issue is when a Property is a dependency on a DerivedProperty AND also has a listener that disposes that DerivedProperty. Here is a simple case that causes the assertion:

const prop = new Property( false );
const derProp = DerivedProperty.not( prop );

prop.lazyLink( () => {
  derProp.dispose();
} );

// Swap the listeners from their default order (so dispose happens before DerivedProperty update
prop.tinyProperty.listeners = new Set( Array.from( prop.tinyProperty.listeners ).reverse() );

// This will assert out that the derProp doesn't have any dependencies because it has already been disposed
prop.value = true; 

Triggering: https://github.com/phetsims/axon/blob/eb190c8159265f88f52cfc4a8c8a1d51509970fa/js/DerivedProperty.ts#L140

Solutions:

1. When CT showes this assertion in the future, add hasListenerOrderDependencies: true to each case, like was done in https://github.com/phetsims/scenery/commit/5f13ff0a0c31c51696282efea377952455b3c0bf over in https://github.com/phetsims/expression-exchange/issues/161
2. Remove the assertion. I'm not sure that would solve anything though, it was added by @jonathanolson in https://github.com/phetsims/axon/commit/11f79f181b3b9305aeacf036cdee3fa37285d713 and should likely stay.
3. Add grace to already disposed DerivedProperties like this patch:

   
   Subject: [PATCH] add Disposable to EnabledComponent, https://github.com/phetsims/expression-exchange/issues/161
   ---
   Index: js/DerivedProperty.ts
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git a/js/DerivedProperty.ts b/js/DerivedProperty.ts
   --- a/js/DerivedProperty.ts (revision 92e4db211f0bbe2c38605c9e4ab4158ea886b1d6)
   +++ b/js/DerivedProperty.ts (date 1680277021146)
   @@ -144,6 +144,10 @@
   // for bind
   private getDerivedPropertyListener(): void {

• if ( this.isDisposed ) {
• return;
• }

• // Just mark that there is a deferred value, then calculate the derivation below when setDeferred() is called. // This is in part supported by the PhET-iO state engine because it can account for intermediate states, such // that this Property won't notify until after it is undeferred and has taken its final value.

I would like to proceed with (3) (or something like it),

@jonathanolson, could you please comment on if that feels acceptable to you (no-op recomputation if the derivedProperty is disposed)? It makes sense to me and solves the order dependency.

Noting this came from good conversation with @samreid. Thanks!

• [ ] This issue is now responsible for the usage of hasListenerOrderDependencies in PressListener, please remove it if we go with a different strategy.

[jonathanolson]

I don't think we should be recomputing/updating disposed Properties, including DerivedProperties. Patched above, and it resolves this issue. @zepumph thoughts/review?

[zepumph]

Oh my goodness wow. Thanks for the commit and I love it. It means I can remove my workaround (committed above).

[zepumph]

The last piece for me is that on Friday @samreid and I spoke, and it felt like he was less convinced that this was the best solution. Sorry if I have misspoken, but @samreid would comment and feel free to close now that @jonathanolson has added his thoughts.

[marlitas]

@samreid and I looked at this, and determined that @jonathanolson's fix is sufficient and that no more work needs to be done here. Closing.