Loading/error state does not work when different bindings use the same expression

[apedroferreira]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Steps to reproduce 🕹

Steps:

1. Run backend-basic test fixture
2. Place a new Text component on the canvas
3. Set the new element's value binding to the expression alwaysLoading.data, which is already being used by a parameter in the propagatedLoading query
4. Text element does not show a loading state as it should, but its default value instead

Additionally:

5. Delete propagatedLoading query
6. Text element correctly shows its loading state

Current behavior 😯

When different bindings use the same expression, the loading/error state only works correctly for one of the bindings.

Expected behavior 🤔

When different bindings use the same expression, the loading/error state works correctly for all bindings.

Context 🔦

No response

Your environment 🌎

No response

[apedroferreira]

Ran into this weird bug, I thought it was an issue with nested bindings but it seems to be more general after all. It should have something to do with the logic in computationStatuses.

[Janpot]

Looks like we're not tracking binding dependencies correctly when we're reusing cached evaluations. This seems to work for me:

diff --git a/packages/toolpad-app/src/runtime/evalJsBindings.ts b/packages/toolpad-app/src/runtime/evalJsBindings.ts
index eb21013c3..cf2cb166f 100644
--- a/packages/toolpad-app/src/runtime/evalJsBindings.ts
+++ b/packages/toolpad-app/src/runtime/evalJsBindings.ts
@@ -133,7 +133,10 @@ export default function evalJsBindings(
     }
   }

-  const computationStatuses = new Map<string, { result: null | BindingEvaluationResult }>();
+  const computationStatuses = new Map<
+    string,
+    { result: null | BindingEvaluationResult; dependencies?: Set<string> }
+  >();
   let currentParentBinding: string | undefined;
   const dependencies: Dependencies = new Map();

@@ -164,6 +167,9 @@ export default function evalJsBindings(
       const computed = computationStatuses.get(expression);
       if (computed) {
         if (computed.result) {
+          if (computed.dependencies) {
+            dependencies.set(bindingId, new Set(computed.dependencies));
+          }
           // From cache
           return computed.result;
         }
@@ -177,7 +183,7 @@ export default function evalJsBindings(
       currentParentBinding = bindingId;
       const result = jsRuntime.evaluateExpression(expression, proxiedScope);
       currentParentBinding = prevContext;
-      computationStatuses.set(expression, { result });
+      computationStatuses.set(expression, { result, dependencies: dependencies.get(bindingId) });
       // From freshly computed
       return result;
     }

We should add a test first before merging this issue though.