Fix patch asymmetry when an Input is patched but the Output is unchanged

* Update handleThisCallback running based off of if the callback was
  written by patch instead of just occuring in the chunk path

Author: Aaron-Wrote-This

dash/dash-renderer/src/actions/dependencies.js

@@ -37,7 +37,7 @@ import {
     resolveDeps
 } from './dependencies_ts';
 import {computePaths, getPath} from './paths';
-import {isCarriedOverByPatch} from './patchAnalysis';
+import {isCarriedOverByPatch, wasWrittenByPatch} from './patchAnalysis';
 
 import {crawlLayout} from './utils';
 
@@ -1265,6 +1265,10 @@ export function getWatchedKeys(id, newProps, graphs) {
  *   chunk count as having changed
  * opts.patchAnalysis: what the `Patch()` operations that produced this chunk
  *   changed. Only the components the patch created get their initial call
+ *   It also allows an input the patch wrote directly to bypass the chunkPath
+ *   dedup, when that input's own component was carried over (so
+ *   its own initial call stays suppressed) but downstream callbacks still
+ *   need to see the new value
  *   Absent when the chunk is not the result of a patch
  *
  * Returns an array of objects:
@@ -1353,23 +1357,36 @@ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) {
             }
         }
         if (!outputsOnly && inIdCallbacks) {
+            const idStr = stringifyId(id);
             const maybeAddCallback = removedArrayInputsOnly
-                ? addCallbackIfArray(stringifyId(id))
+                ? addCallbackIfArray(idStr)
                 : addCallback;
-            let handleThisCallback = maybeAddCallback;
-            if (chunkPath) {
-                handleThisCallback = cb => {
-                    if (
-                        !all(
-                            startsWith(chunkPath),
-                            pluck('path', flatten(cb.getOutputs(paths)))
-                        )
-                    ) {
-                        maybeAddCallback(cb);
-                    }
-                };
-            }
             for (const property in inIdCallbacks) {
+                // A callback, whose outputs are all inside the chunk, is
+                // normally dropped here on the assumption that the
+                // output handling above already covers it
+                // That assumption fails when the patch
+                // wrote a new value directly on this input without
+                // recreating the input's own component
+                // The output side stays suppressed because the output
+                // component was carried over, but this input's value
+                // genuinely changed, so the callback must still be added
+                let handleThisCallback = maybeAddCallback;
+                if (
+                    chunkPath &&
+                    !wasWrittenByPatch(patchAnalysis, idStr, property)
+                ) {
+                    handleThisCallback = cb => {
+                        if (
+                            !all(
+                                startsWith(chunkPath),
+                                pluck('path', flatten(cb.getOutputs(paths)))
+                            )
+                        ) {
+                            maybeAddCallback(cb);
+                        }
+                    };
+                }
                 getCallbacksByInput(
                     graphs,
                     paths,

dash/dash-renderer/src/actions/patchAnalysis.ts

@@ -93,3 +93,21 @@ export function isUntouchedByPatch(
     }
     return !analysis.freshIds[idStr] && !analysis.writtenProps[idStr];
 }
+
+/*
+ * Did the patch write directly on this prop of a component it did not
+ * recreate? This can be true for a component whose own initial call stays
+ * suppressed by `isCarriedOverByPatch`, it's not fresh, even though one of
+ * its props changed, callbacks that depend on that prop as an Input
+ * still need to run
+ */
+export function wasWrittenByPatch(
+    analysis: PatchAnalysis | undefined,
+    idStr: string | undefined | null,
+    property: string
+): boolean {
+    if (!analysis || !idStr) {
+        return false;
+    }
+    return Boolean(analysis.writtenProps[idStr]?.[property]);
+}

dash/dash-renderer/tests/dependencies.test.js

@@ -1,6 +1,10 @@
 import {expect} from 'chai';
 import {beforeEach, describe, it} from 'mocha';
-import {computeGraphs, getAnyVals} from '../src/actions/dependencies';
+import {
+    computeGraphs,
+    getAnyVals,
+    getUnfilteredLayoutCallbacks
+} from '../src/actions/dependencies';
 import {getCallbacksByInput} from '../src/actions/dependencies_ts';
 import {EventEmitter} from '../src/actions/utils';
 
@@ -234,3 +238,98 @@ describe('dependencies — MATCH trigger resolvedId (#2462)', () => {
         expect(second[0].resolvedId).to.include('btn-2');
     });
 });
+
+describe('dependencies: getUnfilteredLayoutCallbacks with a Patch (#3938)', () => {
+    // Create a layout with two elements
+    //      num: Input which patch writes to `value` directly
+    //      badge: Output which `children` are written to, but unchanged by the patch
+
+    // A callback listens to Input(num.value) and writes Output(badge.children)
+    // Both the input and the output live inside the same patched chunk
+    function makeGraphsAndPaths() {
+        const errors = [];
+        const graphs = computeGraphs(
+            [
+                {
+                    output: 'badge.children',
+                    inputs: [{id: 'num', property: 'value'}],
+                    state: [],
+                    no_output: false
+                }
+            ],
+            (m, l) => errors.push({m, l}),
+            config
+        );
+        expect(errors).to.eql([]);
+        const paths = makePaths(['container', 'num', 'badge']);
+        const layoutChunk = {
+            props: {
+                id: 'container',
+                children: [
+                    {props: {id: 'num', value: 100}},
+                    {props: {id: 'badge', children: 'badge: stale'}}
+                ]
+            }
+        };
+        return {graphs, paths, layoutChunk};
+    }
+
+    it('keeps a callback alive when the patch wrote its Input directly, even though its Output was carried over', () => {
+        const {graphs, paths, layoutChunk} = makeGraphsAndPaths();
+
+        const patchAnalysis = {
+            patchedProps: {children: true},
+            freshIds: {},
+            writtenProps: {num: {value: true}}
+        };
+
+        const callbacks = getUnfilteredLayoutCallbacks(
+            graphs,
+            paths,
+            layoutChunk,
+            {chunkPath: ['props', 'children'], patchAnalysis}
+        );
+
+        expect(callbacks).to.have.lengthOf(1);
+        expect(callbacks[0].resolvedId).to.equal('badge.children');
+    });
+
+    it('still drops a carried over callback when neither its Input nor Output was touched by the patch', () => {
+        const {graphs, paths, layoutChunk} = makeGraphsAndPaths();
+
+        const patchAnalysis = {
+            patchedProps: {children: true},
+            freshIds: {},
+            writtenProps: {}
+        };
+
+        const callbacks = getUnfilteredLayoutCallbacks(
+            graphs,
+            paths,
+            layoutChunk,
+            {chunkPath: ['props', 'children'], patchAnalysis}
+        );
+
+        expect(callbacks).to.have.lengthOf(0);
+    });
+
+    it('still runs the callback via its Output when the Output component is fresh', () => {
+        const {graphs, paths, layoutChunk} = makeGraphsAndPaths();
+
+        const patchAnalysis = {
+            patchedProps: {children: true},
+            freshIds: {badge: true},
+            writtenProps: {}
+        };
+
+        const callbacks = getUnfilteredLayoutCallbacks(
+            graphs,
+            paths,
+            layoutChunk,
+            {chunkPath: ['props', 'children'], patchAnalysis}
+        );
+
+        expect(callbacks).to.have.lengthOf(1);
+        expect(callbacks[0].resolvedId).to.equal('badge.children');
+    });
+});