diff --git a/CHANGELOG.md b/CHANGELOG.md index cf94434780..b8ca188cee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,10 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## Fixed - [#3629](https://github.com/plotly/dash/pull/3629) Fix date pickers not showing date when initially rendered in a hidden container. -- [#3627][(](https://github.com/plotly/dash/pull/3627)) Make dropdowns searchable wheen focused, without requiring to open them first +- [#3627][(](https://github.com/plotly/dash/pull/3627)) Make dropdowns searchable when focused, without requiring to open them first - [#3656][(](https://github.com/plotly/dash/pull/3656)) Improved dropdown performance for large collections of options - [#3660][(](https://github.com/plotly/dash/pull/3660)) Allow same date to be selected for both start and end in DatePickerRange components +- [#3682](https://github.com/plotly/dash/pull/3682) Fix initial callbacks when created via dcc.Patch diff --git a/dash/dash-renderer/src/actions/callbacks.ts b/dash/dash-renderer/src/actions/callbacks.ts index ee3a251378..1a4c230b63 100644 --- a/dash/dash-renderer/src/actions/callbacks.ts +++ b/dash/dash-renderer/src/actions/callbacks.ts @@ -41,7 +41,7 @@ import {createAction, Action} from 'redux-actions'; import {addHttpHeaders} from '../actions'; import {notifyObservers, updateProps} from './index'; import {CallbackJobPayload} from '../reducers/callbackJobs'; -import {parsePatchProps} from './patch'; +import {isPatch, parsePatchProps} from './patch'; import {computePaths, getPath} from './paths'; import {requestDependencies} from './requestDependencies'; @@ -826,6 +826,7 @@ export function executeCallback( ); // Patch methodology: always run through parsePatchProps for each output const currentLayout = getState().layout; + let wasPatch = false; flatten(outputs).forEach((out: any) => { const propName = cleanOutputProp(out.property); const outputPath = getPath(paths, out.id); @@ -834,6 +835,9 @@ export function executeCallback( if (outputValue === undefined) { return; } + if (isPatch(outputValue)) { + wasPatch = true; + } const oldProps = path( outputPath.concat(['props']), @@ -849,7 +853,7 @@ export function executeCallback( data ); }); - return {data, payload}; + return {data, payload, ...(wasPatch ? {prePatchPaths: paths} : {})}; } catch (error: any) { return {error, payload}; } @@ -909,6 +913,7 @@ export function executeCallback( // Layout may have changed. // DRY: Always run through parsePatchProps for each output const currentLayout = getState().layout; + let wasPatch = false; flatten(outputs).forEach((out: any) => { const propName = cleanOutputProp(out.property); const outputPath = getPath(paths, out.id); @@ -917,6 +922,9 @@ export function executeCallback( if (outputValue === undefined) { return; } + if (isPatch(outputValue)) { + wasPatch = true; + } const oldProps = path( outputPath.concat(['props']), @@ -941,7 +949,7 @@ export function executeCallback( ); } - return {data, payload}; + return {data, payload, ...(wasPatch ? {prePatchPaths: paths} : {})}; } catch (res: any) { lastError = res; if ( diff --git a/dash/dash-renderer/src/actions/dependencies.js b/dash/dash-renderer/src/actions/dependencies.js index 657647da7e..a3ee012a41 100644 --- a/dash/dash-renderer/src/actions/dependencies.js +++ b/dash/dash-renderer/src/actions/dependencies.js @@ -1194,7 +1194,14 @@ export function getWatchedKeys(id, newProps, graphs) { * See getCallbackByOutput for details. */ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) { - const {outputsOnly, removedArrayInputsOnly, newPaths, chunkPath} = opts; + const { + outputsOnly, + removedArrayInputsOnly, + newPaths, + chunkPath, + oldPaths, + oldLayout + } = opts; const foundCbIds = {}; const callbacks = []; @@ -1240,7 +1247,25 @@ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) { }); } - function handleOneId(id, outIdCallbacks, inIdCallbacks) { + // Suppress initial call only when the component's output prop is unchanged + // (the component was preserved by a Patch operation). Full-replace + // returns new component instances whose props are reset to defaults, so their + // output props will differ from the old layout and the callback must still fire. + function isUnchangedOutputProp(id, property, child) { + if (!chunkPath || !oldPaths || !oldLayout) return false; + const oldPath = getPath(oldPaths, id); + if (!oldPath) return false; + const oldPropValue = path([...oldPath, 'props', property], oldLayout); + // If the prop was never set in the old layout (undefined), we cannot + // treat it as "unchanged" and the callback must still fire to populate it. + // This prevents incorrectly suppressing title callbacks and other output + // props that were undefined in both old and new layouts (undefined === undefined). + if (oldPropValue === undefined) return false; + const newPropValue = child ? path(['props', property], child) : undefined; + return equals(oldPropValue, newPropValue); + } + + function handleOneId(id, outIdCallbacks, inIdCallbacks, child) { if (outIdCallbacks) { for (const property in outIdCallbacks) { const cb = getCallbackByOutput(graphs, paths, id, property); @@ -1249,7 +1274,10 @@ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) { // unless specifically requested not to. // ie this is the initial call of this callback even if it's // not the page initialization but just a new layout chunk - if (!cb.callback.prevent_initial_call) { + if ( + !cb.callback.prevent_initial_call && + !isUnchangedOutputProp(id, property, child) + ) { cb.initialCall = true; addCallback(cb); } @@ -1289,13 +1317,14 @@ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) { const id = path(['props', 'id'], child); if (id) { if (typeof id === 'string' && !removedArrayInputsOnly) { - handleOneId(id, graphs.outputMap[id], graphs.inputMap[id]); + handleOneId(id, graphs.outputMap[id], graphs.inputMap[id], child); } else { const keyStr = Object.keys(id).sort().join(','); handleOneId( id, !removedArrayInputsOnly && graphs.outputPatterns[keyStr], - graphs.inputPatterns[keyStr] + graphs.inputPatterns[keyStr], + child ); } } diff --git a/dash/dash-renderer/src/observers/executedCallbacks.ts b/dash/dash-renderer/src/observers/executedCallbacks.ts index 85d97299bc..44db499e5b 100644 --- a/dash/dash-renderer/src/observers/executedCallbacks.ts +++ b/dash/dash-renderer/src/observers/executedCallbacks.ts @@ -93,7 +93,7 @@ const observer: IStoreObserverDefinition = { return; } - const {data, error, payload} = executionResult; + const {data, error, payload, prePatchPaths} = executionResult; if (data !== undefined) { Object.entries(data).forEach( @@ -156,10 +156,21 @@ const observer: IStoreObserverDefinition = { dispatch(setPaths(paths)); // Get callbacks for new layout (w/ execution group) + // Only pass oldPaths/oldLayout for Patch callbacks: + // isUnchangedOutputProp must only suppress initial + // calls when a Patch carried over existing components. + // For full-replacement callbacks every component is a + // fresh instance and all initial calls must fire. requestedCallbacks = concat( requestedCallbacks, getLayoutCallbacks(graphs, paths, children, { chunkPath: oldChildrenPath, + ...(prePatchPaths + ? { + oldPaths: oPaths, + oldLayout: oldLayout + } + : {}), filterRoot }).map(rcb => ({ ...rcb, diff --git a/dash/dash-renderer/src/types/callbacks.ts b/dash/dash-renderer/src/types/callbacks.ts index f1e1dc382c..de7853aca5 100644 --- a/dash/dash-renderer/src/types/callbacks.ts +++ b/dash/dash-renderer/src/types/callbacks.ts @@ -83,6 +83,7 @@ export type CallbackResult = { data?: CallbackResponse; error?: Error; payload: ICallbackPayload | null; + prePatchPaths?: any; }; export type BackgroundCallbackInfo = { diff --git a/tests/integration/callbacks/test_wildcards.py b/tests/integration/callbacks/test_wildcards.py index 9fd7337040..1d66dfca5d 100644 --- a/tests/integration/callbacks/test_wildcards.py +++ b/tests/integration/callbacks/test_wildcards.py @@ -1,12 +1,13 @@ import pytest import re +import threading from selenium.webdriver.common.keys import Keys import json from multiprocessing import Lock from dash.testing import wait import dash -from dash import Dash, Input, Output, State, ALL, ALLSMALLER, MATCH, html, dcc +from dash import Dash, Input, Output, State, ALL, ALLSMALLER, MATCH, html, dcc, Patch from tests.assets.todo_app import todo_app from tests.assets.grouping_app import grouping_app @@ -619,3 +620,173 @@ def on_click(_) -> str: assert not dash_duo.find_element("#buttons button:nth-child(2)").get_attribute( "disabled" ) + + +def test_cbwc009_patch_no_spurious_match_callbacks(dash_duo): + """Regression test for the oldPaths fix in getUnfilteredLayoutCallbacks. + + When Patch() appends a new MATCH-pattern component, existing MATCH callbacks + must NOT re-fire for pre-existing components. Previously, crawlLayout would + visit all children in the layout chunk and mark every matching output as + initialCall=true, causing all existing callbacks to spuriously re-execute. + + The fix passes oldPaths (the pre-update paths snapshot) into + getUnfilteredLayoutCallbacks and skips initialCall for any component whose + ID already exists in oldPaths. + """ + lock = threading.Lock() + fire_counts = {} # {index: count} how many times each MATCH callback fired + + def make_item(index): + return html.Div( + [ + dcc.Input( + id={"type": "item-input", "index": index}, + value=index, + type="number", + className="item-input", + ), + html.Div( + "init", + id={"type": "item-output", "index": index}, + className="item-output", + ), + ] + ) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Add", id="add-btn", n_clicks=0), + html.Div([make_item(0), make_item(1)], id="container"), + ] + ) + + @app.callback( + Output("container", "children"), + Input("add-btn", "n_clicks"), + prevent_initial_call=True, + ) + def add_item(n): + p = Patch() + p.append(make_item(n + 1)) + return p + + @app.callback( + Output({"type": "item-output", "index": MATCH}, "children"), + Input({"type": "item-input", "index": MATCH}, "value"), + ) + def on_value_change(value): + from dash import ctx + + idx = ctx.outputs_grouping["id"]["index"] + with lock: + fire_counts[idx] = fire_counts.get(idx, 0) + 1 + count = fire_counts[idx] + return f"fired-{idx}-#{count}" + + dash_duo.start_server(app) + + # Wait for the initial callbacks to fire for both pre-existing items. + wait.until(lambda: fire_counts.get(0, 0) >= 1, 5) + wait.until(lambda: fire_counts.get(1, 0) >= 1, 5) + + counts_before = {0: fire_counts[0], 1: fire_counts[1]} + + # Add a new item via Patch, this should fire only for index 2. + dash_duo.find_element("#add-btn").click() + wait.until(lambda: fire_counts.get(2, 0) >= 1, 5) + + # Pre-existing callbacks must NOT have re-fired. + assert fire_counts[0] == counts_before[0], ( + f"Item 0 callback fired spuriously after Patch: " + f"was {counts_before[0]}, now {fire_counts[0]}" + ) + assert fire_counts[1] == counts_before[1], ( + f"Item 1 callback fired spuriously after Patch: " + f"was {counts_before[1]}, now {fire_counts[1]}" + ) + assert ( + fire_counts[2] == 1 + ), f"New item 2 callback should have fired exactly once, fired {fire_counts[2]}" + + +def test_cbwc010_full_replace_fires_initial_callbacks(dash_duo): + """Regression test ensuring full-replacement (non-Patch) outputs still fire + initial callbacks for all replaced components. + + When a callback returns a full list (not a Patch), every component in the + new layout is a fresh instance. The isUnchangedOutputProp suppression must + NOT apply here: prePatchPaths is absent from the execution result, so + oldPaths/oldLayout are never passed, and all initial calls must fire. + """ + lock = threading.Lock() + fire_counts = {} # {index: count} + + def make_item(index): + return html.Div( + [ + dcc.Input( + id={"type": "fr-input", "index": index}, + value=index, + type="number", + className="fr-input", + ), + html.Div( + "init", + id={"type": "fr-output", "index": index}, + className="fr-output", + ), + ] + ) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Replace", id="replace-btn", n_clicks=0), + html.Div([make_item(0), make_item(1)], id="fr-container"), + ] + ) + + @app.callback( + Output("fr-container", "children"), + Input("replace-btn", "n_clicks"), + prevent_initial_call=True, + ) + def replace_items(_): + # Full replacement — returns a plain list, not a Patch + return [make_item(10), make_item(11)] + + @app.callback( + Output({"type": "fr-output", "index": MATCH}, "children"), + Input({"type": "fr-input", "index": MATCH}, "value"), + ) + def on_value_change(value): + from dash import ctx + + idx = ctx.outputs_grouping["id"]["index"] + with lock: + fire_counts[idx] = fire_counts.get(idx, 0) + 1 + count = fire_counts[idx] + return f"fired-{idx}-#{count}" + + dash_duo.start_server(app) + + # Wait for the initial callbacks for items 0 and 1. + wait.until(lambda: fire_counts.get(0, 0) >= 1, 5) + wait.until(lambda: fire_counts.get(1, 0) >= 1, 5) + + # Trigger a full replacement. + dash_duo.find_element("#replace-btn").click() + + # After full replacement, items 10 and 11 are brand-new instances and + # MUST have their initial callbacks fire. + wait.until(lambda: fire_counts.get(10, 0) >= 1, 5) + wait.until(lambda: fire_counts.get(11, 0) >= 1, 5) + + assert ( + fire_counts[10] >= 1 + ), f"New item 10 callback should have fired after full replace, got {fire_counts.get(10, 0)}" + assert ( + fire_counts[11] >= 1 + ), f"New item 11 callback should have fired after full replace, got {fire_counts.get(11, 0)}"