diff --git a/CHANGELOG.md b/CHANGELOG.md index d3b77ae064..d24e7b86ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ### Fixed - [#3916](https://github.com/plotly/dash/pull/3916) Fixed a regression where dragging multiple files into `dcc.Upload` would upload only the first file when `multiple=True` - [#3922](https://github.com/plotly/dash/pull/3922) Fix `dcc.Input(type="number")` stepper behavior when only `min` is set. +- [#???](https://github.com/plotly/dash/pull/???) Fix `dcc.Patch()` re-running the initial callbacks of components that were already on the page, including every matching (`MATCH`/`ALL`) element, and wiping their user-edited persisted values. Fixes [#3681](https://github.com/plotly/dash/issues/3681) and [#3937](https://github.com/plotly/dash/issues/3937) ## [4.4.1] - 2026-07-21 diff --git a/dash/dash-renderer/src/actions/callbacks.ts b/dash/dash-renderer/src/actions/callbacks.ts index 6dec01996c..a5a5c46fc4 100644 --- a/dash/dash-renderer/src/actions/callbacks.ts +++ b/dash/dash-renderer/src/actions/callbacks.ts @@ -32,6 +32,7 @@ import { BackgroundCallbackInfo, CallbackResponse, CallbackResponseData, + PatchedOutputs, SideUpdateOutput } from '../types/callbacks'; import {isMultiValued, stringifyId, isMultiOutputProp} from './dependencies'; @@ -41,7 +42,8 @@ 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 {createPatchAnalysis} from './patchAnalysis'; import {computePaths, getPath} from './paths'; import {requestDependencies} from './requestDependencies'; @@ -246,6 +248,10 @@ function cleanOutputProp(property: string) { return property.split('@')[0]; } +function patchedResultFields(patchedOutputs: PatchedOutputs) { + return keys(patchedOutputs).length ? {patchedOutputs} : {}; +} + async function handleClientside( dispatch: any, clientside_function: any, @@ -970,14 +976,23 @@ export function executeCallback( ); // Patch methodology: always run through parsePatchProps for each output const currentLayout = getState().layout; + const patchedOutputs: PatchedOutputs = {}; flatten(outputs).forEach((out: any) => { const propName = cleanOutputProp(out.property); const outputPath = getPath(paths, out.id); - const dataPath = [stringifyId(out.id), propName]; + const idStr = stringifyId(out.id); + const dataPath = [idStr, propName]; const outputValue = path(dataPath, data); if (outputValue === undefined) { return; } + if (isPatch(outputValue)) { + // One analysis per output, shared by all of its + // patched props + patchedOutputs[idStr] = + patchedOutputs[idStr] || + createPatchAnalysis(); + } const oldProps = path( outputPath.concat(['props']), @@ -985,7 +1000,8 @@ export function executeCallback( ) || {}; const newProps = parsePatchProps( {[propName]: outputValue}, - oldProps + oldProps, + patchedOutputs[idStr] ); data = assocPath( dataPath, @@ -993,7 +1009,11 @@ export function executeCallback( data ); }); - return {data, payload}; + return { + data, + payload, + ...patchedResultFields(patchedOutputs) + }; } catch (error: any) { return {error, payload}; } @@ -1079,14 +1099,23 @@ export function executeCallback( // Layout may have changed. // DRY: Always run through parsePatchProps for each output const currentLayout = getState().layout; + const patchedOutputs: PatchedOutputs = {}; flatten(outputs).forEach((out: any) => { const propName = cleanOutputProp(out.property); const outputPath = getPath(paths, out.id); - const dataPath = [stringifyId(out.id), propName]; + const idStr = stringifyId(out.id); + const dataPath = [idStr, propName]; const outputValue = path(dataPath, data); if (outputValue === undefined) { return; } + if (isPatch(outputValue)) { + // One analysis per output, shared by all of its + // patched props + patchedOutputs[idStr] = + patchedOutputs[idStr] || + createPatchAnalysis(); + } const oldProps = path( outputPath.concat(['props']), @@ -1094,7 +1123,8 @@ export function executeCallback( ) || {}; const newProps = parsePatchProps( {[propName]: outputValue}, - oldProps + oldProps, + patchedOutputs[idStr] ); data = assocPath( @@ -1111,7 +1141,11 @@ export function executeCallback( ); } - return {data, payload}; + return { + data, + payload, + ...patchedResultFields(patchedOutputs) + }; } 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 fa29199a1d..0d0ce09526 100644 --- a/dash/dash-renderer/src/actions/dependencies.js +++ b/dash/dash-renderer/src/actions/dependencies.js @@ -37,6 +37,7 @@ import { resolveDeps } from './dependencies_ts'; import {computePaths, getPath} from './paths'; +import {isCarriedOverByPatch} from './patchAnalysis'; import {crawlLayout} from './utils'; @@ -1262,13 +1263,22 @@ export function getWatchedKeys(id, newProps, graphs) { * opts.chunkPath: path to the new chunk - used to determine if any outputs are * outside of this chunk, because this determines whether inputs inside the * 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 + * Absent when the chunk is not the result of a patch * * Returns an array of objects: * {callback, resolvedId, getOutputs, getInputs, getState, ...etc} * See getCallbackByOutput for details. */ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) { - const {outputsOnly, removedArrayInputsOnly, newPaths, chunkPath} = opts; + const { + outputsOnly, + removedArrayInputsOnly, + newPaths, + chunkPath, + patchAnalysis + } = opts; const foundCbIds = {}; const callbacks = []; @@ -1316,6 +1326,18 @@ export function getUnfilteredLayoutCallbacks(graphs, paths, layoutChunk, opts) { function handleOneId(id, outIdCallbacks, inIdCallbacks) { if (outIdCallbacks) { + // Suppress the initial call for components a Patch carried over + // The patch itself tells us which components it created, including + // components rebuilt with an id that was already in use, + // whose initial callbacks must run again even if their new defaults + // happen to match the values of the instance they replaced. + // It excludes the containers between the patched prop and the value + // that changed: ramda's assocPath has to rebuild those, but the + // patch did not create them, so they keep their initial call + // suppressed + const isCarryOver = patchAnalysis + ? isCarriedOverByPatch(patchAnalysis, stringifyId(id)) + : false; for (const property in outIdCallbacks) { const cb = getCallbackByOutput(graphs, paths, id, property); if (cb) { @@ -1323,7 +1345,7 @@ 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 && !isCarryOver) { cb.initialCall = true; addCallback(cb); } diff --git a/dash/dash-renderer/src/actions/patch.ts b/dash/dash-renderer/src/actions/patch.ts index 3c3e14d204..5f91b6d4c2 100644 --- a/dash/dash-renderer/src/actions/patch.ts +++ b/dash/dash-renderer/src/actions/patch.ts @@ -13,6 +13,10 @@ import { reverse } from 'ramda'; +import {stringifyId} from './dependencies'; +import {isDryComponent} from '../wrapper/wrapping'; +import {PatchAnalysis} from './patchAnalysis'; + type PatchOperation = { operation: string; location: LocationIndex[]; @@ -288,7 +292,108 @@ const patchHandlers: {[k: string]: PatchHandler} = { } }; -export function handlePatch(previousValue: T, patchValue: any): T { +/* + * Operations that put a value into the tree, which add/remove ids, + * and need to be handled during a patch + */ +const insertingOperations: {[operation: string]: true} = { + Assign: true, + Merge: true, + Extend: true, + Insert: true, + Append: true, + Prepend: true +}; + +function collectComponentIds( + value: any, + freshIds: PatchAnalysis['freshIds'], + visited: Set +) { + if (!value || typeof value !== 'object' || visited.has(value)) { + return; + } + visited.add(value); + + if (Array.isArray(value)) { + value.forEach(item => collectComponentIds(item, freshIds, visited)); + return; + } + + if (isDryComponent(value)) { + const {id} = value.props; + if (id !== undefined && id !== null) { + freshIds[stringifyId(id)] = true; + } + collectComponentIds(value.props, freshIds, visited); + return; + } + + for (const key in value) { + collectComponentIds(value[key], freshIds, visited); + } +} + +function recordWrittenProp( + previous: any, + location: LocationIndex[], + writtenProps: PatchAnalysis['writtenProps'] +) { + let current = previous; + let idStr: string | null = null; + let property: string | null = null; + + for (let i = 0; i < location.length && current; i++) { + const key = location[i]; + if ( + key === 'props' && + i + 1 < location.length && + isDryComponent(current) && + current.props.id !== undefined && + current.props.id !== null + ) { + idStr = stringifyId(current.props.id); + property = String(location[i + 1]); + } + current = current[key]; + } + + if (idStr !== null && property !== null) { + const props = writtenProps[idStr] || (writtenProps[idStr] = {}); + props[property] = true; + } +} + +function recordPatchOperation( + previous: any, + patchOperation: PatchOperation, + analysis: PatchAnalysis +) { + const {operation, location, params} = patchOperation; + + if (insertingOperations[operation]) { + collectComponentIds(params.value, analysis.freshIds, new Set()); + } + + if (operation === 'Merge' && params.value && is(Object, params.value)) { + Object.keys(params.value).forEach(key => + recordWrittenProp( + previous, + location.concat(key), + analysis.writtenProps + ) + ); + return; + } + + recordWrittenProp(previous, location, analysis.writtenProps); +} + +export function handlePatch( + previousValue: T, + patchValue: any, + analysis?: PatchAnalysis +): T { let reducedValue = previousValue; for (let i = 0; i < patchValue.operations.length; i++) { @@ -298,15 +403,24 @@ export function handlePatch(previousValue: T, patchValue: any): T { if (!handler) { throw new Error(`Invalid Operation ${patch.operation}`); } + if (analysis) { + recordPatchOperation(reducedValue, patch, analysis); + } reducedValue = handler(reducedValue, patch); } return reducedValue; } +/* + * `analysis`, when provided, is filled in with what the patches did. + * Props that are not patches are left out of it, so callers + * can tell a patched prop from a fully replaced one + */ export function parsePatchProps( props: any, - previousProps: any + previousProps: any, + analysis?: PatchAnalysis ): Record { if (!is(Object, props)) { return props; @@ -321,7 +435,10 @@ export function parsePatchProps( if (previousValue === undefined) { throw new Error('Cannot patch undefined'); } - patchedProps[key] = handlePatch(previousValue, val); + if (analysis) { + analysis.patchedProps[key] = true; + } + patchedProps[key] = handlePatch(previousValue, val, analysis); } else { patchedProps[key] = val; } diff --git a/dash/dash-renderer/src/actions/patchAnalysis.ts b/dash/dash-renderer/src/actions/patchAnalysis.ts new file mode 100644 index 0000000000..d882e7b968 --- /dev/null +++ b/dash/dash-renderer/src/actions/patchAnalysis.ts @@ -0,0 +1,95 @@ +/* + * "What did the patch actually change?" + * + * Patches are applied with ramda's `assocPath`/`dissocPath`, which + * shallow-clone every object along the path they touch. So after a patch the + * only thing reference identity tells us is "this object is not on the path + * the patch walked". It cannot distinguish a component the patch created from + * a container that merely sits between the patched prop and the value that + * changed, because both end up with a brand new `props` object + * + * * `freshIds`: components the patch inserted into the tree. New + * instances even when they reuse an id that was already on the page, so + * their initial callbacks must run and their persisted user edits must be + * restored + * * `writtenProps`: props the patch wrote directly on a component that already + * existed (`p[0]['props']['value'] = 5`). The component was not recreated, + * but the server did provide a new value for that prop, which persistence + * needs to know about to detect a server override + * + * Everything else in the resulting tree was carried over from the previous + * layout, whatever its `props` reference says + */ +export type PatchAnalysis = { + /* + * Props of the output component that were produced by a Patch. The rest of + * this analysis only describes those props: other props of the same output + * are regular values which fully replace what was there + */ + patchedProps: {[property: string]: true}; + /* Stringified ids of the components the patch inserted. */ + freshIds: {[idStr: string]: true}; + /* Props the patch wrote on components that already existed. */ + writtenProps: {[idStr: string]: {[property: string]: true}}; +}; + +export function createPatchAnalysis(): PatchAnalysis { + return {patchedProps: {}, freshIds: {}, writtenProps: {}}; +} + +/* + * The analysis, but only if `property` is one of the props it describes. + * A callback can return a Patch for one prop and a full value for another, and + * a full value is a fresh sub-tree in which nothing was carried over + */ +export function analysisForProp( + analysis: PatchAnalysis | undefined, + property: string +): PatchAnalysis | undefined { + return analysis && analysis.patchedProps[property] ? analysis : undefined; +} + +/* + * For consumers that look at several props of an output at once, the + * analysis only applies if every one of them came from a Patch + */ +export function analysisForAllProps( + analysis: PatchAnalysis | undefined, + properties: (string | number)[] +): PatchAnalysis | undefined { + return analysis && + properties.length && + properties.every(property => analysis.patchedProps[property]) + ? analysis + : undefined; +} + +/* + * Was this component already in the layout before the patch? + * Without an analysis the chunk did not come from a patch, so + * nothing was carried over, so every component is new + */ +export function isCarriedOverByPatch( + analysis: PatchAnalysis | undefined, + idStr: string | undefined | null +): boolean { + if (!analysis || !idStr) { + return false; + } + return !analysis.freshIds[idStr]; +} + +/* + * Stricter version of `isCarriedOverByPatch` for persistence where a component + * was kept and the patch did not write any prop on. These components still hold + * user edits in Redux, so persistence must leave it alone + */ +export function isUntouchedByPatch( + analysis: PatchAnalysis | undefined, + idStr: string | undefined | null +): boolean { + if (!analysis || !idStr) { + return false; + } + return !analysis.freshIds[idStr] && !analysis.writtenProps[idStr]; +} diff --git a/dash/dash-renderer/src/observers/executedCallbacks.ts b/dash/dash-renderer/src/observers/executedCallbacks.ts index 85d97299bc..7cdd7239a0 100644 --- a/dash/dash-renderer/src/observers/executedCallbacks.ts +++ b/dash/dash-renderer/src/observers/executedCallbacks.ts @@ -37,6 +37,11 @@ import {ICallback, IStoredCallback} from '../types/callbacks'; import {updateProps, setPaths, handleAsyncError} from '../actions'; import {getPath, computePaths} from '../actions/paths'; +import { + PatchAnalysis, + analysisForAllProps, + analysisForProp +} from '../actions/patchAnalysis'; import {applyPersistence, prunePersistence} from '../persistence'; import {IStoreObserverDefinition} from '../StoreObserver'; @@ -47,7 +52,11 @@ const observer: IStoreObserverDefinition = { callbacks: {executed} } = getState(); - function applyProps(id: any, updatedProps: any) { + function applyProps( + id: any, + updatedProps: any, + patchAnalysis?: PatchAnalysis + ) { const {layout, paths} = getState(); const itempath = getPath(paths, id); if (!itempath) { @@ -65,7 +74,20 @@ const observer: IStoreObserverDefinition = { // In case the update contains whole components, see if any of // those components have props to update to persist user edits. - const {props} = applyPersistence({props: updatedProps}, dispatch); + // A Patch resolves by carrying pre-existing children over from + // Redux, user edits included, so applyPersistence must leave those + // alone. It would otherwise see a "server override" and clear the + // stored edit. The analysis says which components the patch really + // created. Everything else came from a full replacement, where the + // server returns fresh default values and persisted edits must be + // restored (e.g. after a component moves on page). + // Only the `children` prop matters here, that is the one + // applyPersistence recurses through + const {props} = applyPersistence( + {props: updatedProps}, + dispatch, + analysisForProp(patchAnalysis, 'children') + ); (dispatch as ThunkDispatch)( updateProps({ itempath, @@ -93,7 +115,7 @@ const observer: IStoreObserverDefinition = { return; } - const {data, error, payload} = executionResult; + const {data, error, payload, patchedOutputs} = executionResult; if (data !== undefined) { Object.entries(data).forEach( @@ -105,8 +127,15 @@ const observer: IStoreObserverDefinition = { paths: oldPaths } = getState(); + // What the Patch operations of this output changed + const patchAnalysis = patchedOutputs?.[id]; + // Components will trigger callbacks on their own as required (eg. derived) - const appliedProps = applyProps(parsedId, props); + const appliedProps = applyProps( + parsedId, + props, + patchAnalysis + ); // Add callbacks for modified inputs requestedCallbacks = concat( @@ -145,7 +174,8 @@ const observer: IStoreObserverDefinition = { children: any, oldChildren: any, oldChildrenPath: any[], - filterRoot: any = false + filterRoot: any = false, + propAnalysis?: PatchAnalysis ) => { const oPaths = getState().paths; const paths = computePaths( @@ -160,6 +190,7 @@ const observer: IStoreObserverDefinition = { requestedCallbacks, getLayoutCallbacks(graphs, paths, children, { chunkPath: oldChildrenPath, + patchAnalysis: propAnalysis, filterRoot }).map(rcb => ({ ...rcb, @@ -211,6 +242,9 @@ const observer: IStoreObserverDefinition = { } // Crawl layout needs the ns/type + // This crawls every prop of the component at + // once, so the analysis only applies if all + // of them came from a Patch handlePaths( { ...oldObj, @@ -221,7 +255,11 @@ const observer: IStoreObserverDefinition = { }, oldObj, basePath, - keys(appliedProps) + keys(appliedProps), + analysisForAllProps( + patchAnalysis, + keys(props) + ) ); // Only do it once for the component. recomputed = true; @@ -248,7 +286,12 @@ const observer: IStoreObserverDefinition = { handlePaths( children, oldChildren, - oldChildrenPath + oldChildrenPath, + false, + analysisForProp( + patchAnalysis, + childrenPropPath[0] + ) ); } }); diff --git a/dash/dash-renderer/src/persistence.js b/dash/dash-renderer/src/persistence.js index a029736866..23ff4f0b61 100644 --- a/dash/dash-renderer/src/persistence.js +++ b/dash/dash-renderer/src/persistence.js @@ -70,6 +70,7 @@ import {createAction} from 'redux-actions'; import Registry from './registry'; import {stringifyId} from './actions/dependencies'; +import {isUntouchedByPatch} from './actions/patchAnalysis'; import {isDryComponent} from './wrapper/wrapping'; export const storePrefix = '_dash_persistence.'; @@ -360,14 +361,19 @@ export function recordUiEdit(layout, newProps, dispatch) { /* * Used for entire layouts (on load) or partial layouts (from children * callbacks) to apply previously-stored UI edits to components + * + * `patchAnalysis` (optional) describes what the `Patch()` that produced this + * layout changed */ -export function applyPersistence(layout, dispatch) { +export function applyPersistence(layout, dispatch, patchAnalysis) { if (Array.isArray(layout)) { return layout.map(lay => - isDryComponent(lay) ? persistenceMods(lay, lay, [], dispatch) : lay + isDryComponent(lay) + ? persistenceMods(lay, lay, [], dispatch, patchAnalysis) + : lay ); } - return persistenceMods(layout, layout, [], dispatch); + return persistenceMods(layout, layout, [], dispatch, patchAnalysis); } const UNDO = true; @@ -392,7 +398,7 @@ function modProp(key, storage, element, props, persistedProp, update, undo) { } } -function persistenceMods(layout, component, path, dispatch) { +function persistenceMods(layout, component, treePath, dispatch, patchAnalysis) { const { canPersist, id, @@ -404,7 +410,18 @@ function persistenceMods(layout, component, path, dispatch) { } = getProps(component); let layoutOut = layout; - if (canPersist && persistence) { + + // Skip the components a Patch carried over from the previous layout + // untouched. The patch itself tells us which + // components it created and which props it wrote + // Note that a component rebuilt with an id that was already in use is + // not carried over. It comes with the server's default value, so its + // persisted edit must be restored + const carriedOver = patchAnalysis + ? isUntouchedByPatch(patchAnalysis, id && stringifyId(id)) + : false; + + if (canPersist && persistence && !carriedOver) { const storage = getStore(persistence_type, dispatch); const update = {}; forEach( @@ -422,7 +439,7 @@ function persistenceMods(layout, component, path, dispatch) { for (const propName in update) { layoutOut = set( - lensPath(path.concat('props', propName)), + lensPath(treePath.concat('props', propName)), update[propName], layoutOut ); @@ -437,8 +454,9 @@ function persistenceMods(layout, component, path, dispatch) { layoutOut = persistenceMods( layoutOut, child, - path.concat('props', 'children', i), - dispatch + treePath.concat('props', 'children', i), + dispatch, + patchAnalysis ); } }); @@ -446,8 +464,9 @@ function persistenceMods(layout, component, path, dispatch) { layoutOut = persistenceMods( layoutOut, children, - path.concat('props', 'children'), - dispatch + treePath.concat('props', 'children'), + dispatch, + patchAnalysis ); } return layoutOut; diff --git a/dash/dash-renderer/src/types/callbacks.ts b/dash/dash-renderer/src/types/callbacks.ts index 5f963463d2..b5ebf571cd 100644 --- a/dash/dash-renderer/src/types/callbacks.ts +++ b/dash/dash-renderer/src/types/callbacks.ts @@ -1,3 +1,5 @@ +import {PatchAnalysis} from '../actions/patchAnalysis'; + type CallbackId = string | {[key: string]: any}; export interface ICallbackDefinition { @@ -85,6 +87,11 @@ export type CallbackResult = { data?: CallbackResponse; error?: Error; payload: ICallbackPayload | null; + patchedOutputs?: PatchedOutputs; +}; + +export type PatchedOutputs = { + [idStr: string]: PatchAnalysis; }; export type BackgroundCallbackInfo = { diff --git a/dash/dash-renderer/tests/patch.test.js b/dash/dash-renderer/tests/patch.test.js new file mode 100644 index 0000000000..ce89c96a01 --- /dev/null +++ b/dash/dash-renderer/tests/patch.test.js @@ -0,0 +1,186 @@ +import {expect} from 'chai'; +import {describe, it} from 'mocha'; +import {handlePatch, parsePatchProps} from '../src/actions/patch'; +import { + analysisForAllProps, + analysisForProp, + createPatchAnalysis, + isCarriedOverByPatch, + isUntouchedByPatch +} from '../src/actions/patchAnalysis'; +import {stringifyId} from '../src/actions/dependencies'; + +const component = (id, props = {}) => ({ + namespace: 'dash_html_components', + type: 'Div', + props: {id, ...props} +}); + +const patch = (...operations) => ({ + __dash_patch_update: '__dash_patch_update', + operations +}); + +describe('patch analysis, what a Patch changed', () => { + it('reports appended components as created, existing ones as carried over', () => { + const analysis = createPatchAnalysis(); + const children = [component('kept', {value: 'edited'})]; + + const result = handlePatch( + children, + patch({ + operation: 'Append', + location: [], + params: {value: component('added')} + }), + analysis + ); + + expect(result.length).to.equal(2); + expect(analysis.freshIds).to.deep.equal({added: true}); + expect(isCarriedOverByPatch(analysis, 'added')).to.equal(false); + expect(isCarriedOverByPatch(analysis, 'kept')).to.equal(true); + expect(isUntouchedByPatch(analysis, 'kept')).to.equal(true); + }); + + it('reports components rebuilt with a reused id as created', () => { + const analysis = createPatchAnalysis(); + const children = [component('reused', {value: 'edited'})]; + + const result = handlePatch( + children, + patch( + {operation: 'Clear', location: [], params: {}}, + { + operation: 'Append', + location: [], + params: {value: component('reused', {value: 'initial'})} + } + ), + analysis + ); + + expect(result.length).to.equal(1); + expect(analysis.freshIds).to.deep.equal({reused: true}); + expect(isCarriedOverByPatch(analysis, 'reused')).to.equal(false); + expect(isUntouchedByPatch(analysis, 'reused')).to.equal(false); + }); + + it('does not report the containers assocPath had to rebuild as created', () => { + const analysis = createPatchAnalysis(); + const children = [ + component('row', { + children: [component('input', {value: 'edited'})] + }) + ]; + + handlePatch( + children, + patch({ + operation: 'Assign', + location: [0, 'props', 'children', 0, 'props', 'value'], + params: {value: 'server'} + }), + analysis + ); + + expect(analysis.freshIds).to.deep.equal({}); + expect(analysis.writtenProps).to.deep.equal({input: {value: true}}); + // The container between the patched prop and the changed value gets a + // new props object, but the patch did not create it + expect(isCarriedOverByPatch(analysis, 'row')).to.equal(true); + expect(isUntouchedByPatch(analysis, 'row')).to.equal(true); + // The component the patch wrote on is not new either, but persistence + // still has to look at it to notice the server override + expect(isCarriedOverByPatch(analysis, 'input')).to.equal(true); + expect(isUntouchedByPatch(analysis, 'input')).to.equal(false); + }); + + it('records the written prop of a merge on an existing component', () => { + const analysis = createPatchAnalysis(); + const children = [component('input', {value: 'edited'})]; + + handlePatch( + children, + patch({ + operation: 'Merge', + location: [0, 'props'], + params: {value: {value: 'server', className: 'c'}} + }), + analysis + ); + + expect(analysis.freshIds).to.deep.equal({}); + expect(analysis.writtenProps).to.deep.equal({ + input: {value: true, className: true} + }); + }); + + it('collects nested and wildcard ids of created components', () => { + const analysis = createPatchAnalysis(); + const wildcardId = {type: 'row', index: 2}; + + handlePatch( + [], + patch({ + operation: 'Append', + location: [], + params: { + value: component(wildcardId, { + children: [component('deep')] + }) + } + }), + analysis + ); + + expect(analysis.freshIds).to.deep.equal({ + [stringifyId(wildcardId)]: true, + deep: true + }); + }); + + it('does not report removed components as created', () => { + const analysis = createPatchAnalysis(); + const gone = component('gone'); + + handlePatch( + [component('kept'), gone], + patch({operation: 'Remove', location: [], params: {value: gone}}), + analysis + ); + + expect(analysis.freshIds).to.deep.equal({}); + }); + + it('only describes the props that were patched', () => { + const analysis = createPatchAnalysis(); + + const props = parsePatchProps( + { + children: patch({ + operation: 'Append', + location: [], + params: {value: component('added')} + }), + style: {color: 'red'} + }, + {children: [component('kept')], style: {}}, + analysis + ); + + expect(props.children.length).to.equal(2); + expect(analysis.patchedProps).to.deep.equal({children: true}); + expect(analysisForProp(analysis, 'children')).to.equal(analysis); + expect(analysisForProp(analysis, 'style')).to.equal(undefined); + expect(analysisForAllProps(analysis, ['children'])).to.equal(analysis); + expect(analysisForAllProps(analysis, ['children', 'style'])).to.equal( + undefined + ); + }); + + it('treats every component as new without an analysis', () => { + expect(isCarriedOverByPatch(undefined, 'anything')).to.equal(false); + expect(isUntouchedByPatch(undefined, 'anything')).to.equal(false); + }); +}); diff --git a/tests/integration/callbacks/test_wildcards.py b/tests/integration/callbacks/test_wildcards.py index c151e8c277..66d4502e04 100644 --- a/tests/integration/callbacks/test_wildcards.py +++ b/tests/integration/callbacks/test_wildcards.py @@ -1,5 +1,6 @@ import pytest import re +import threading from selenium.webdriver.common.keys import Keys import json from multiprocessing import Lock @@ -16,6 +17,7 @@ MATCH, html, dcc, + Patch, set_props, ) @@ -711,3 +713,413 @@ def announce(_, id_): dash_duo.wait_for_text_to_equal("#out", "clicked index=2") assert dash_duo.get_logs() == [] + + +def test_cbwc011_patch_no_spurious_match_callbacks(dash_duo): + """Test for the initial call suppression in getUnfilteredLayoutCallbacks + + When Patch() appends a new MATCHpattern component, existing MATCH callbacks + must not refire for preexisting components. Previously, crawlLayout would + visit all children in the layout chunk and mark every matching output as + initial Call=true, causing all existing callbacks to spuriously reexecute + + The fix uses what the patch operations recorded while they were applied + to only give an initial call to the components the patch actually created + """ + 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 preexisting 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) + + # Preexisting callbacks must not have refired + 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_cbwc012_patch_no_spurious_match_callbacks_undefined_output_prop(dash_duo): + """Existing MATCH components whose output prop is undefined + must not have their callbacks refired when Patch() appends a new sibling + + This covers components whose output prop has no value at all + (dcc.Slider without an explicit `value`): suppression must not depend on + comparing prop values, which cannot distinguish "unchanged" from "undefined + on both sides", but on whether the patch created the component + """ + lock = threading.Lock() + fire_counts = {} # {aio_id: count} + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Add Slider", id="add-btn", n_clicks=0), + html.Div(id="slider-container", children=[]), + ] + ) + + @app.callback( + Output("slider-container", "children"), + Input("add-btn", "n_clicks"), + prevent_initial_call=True, + ) + def add_slider(n_clicks): + p = Patch() + # dcc.Slider with no explicit `value`, value prop is undefined in layout + p.append( + dcc.Slider( + id={"type": "slider", "aio_id": str(n_clicks)}, + step=0.1, + persistence=str(n_clicks), + persistence_type="local", + ) + ) + return p + + @app.callback( + Output({"type": "slider", "aio_id": MATCH}, "value"), + Input({"type": "slider", "aio_id": MATCH}, "value"), + prevent_initial_call=False, + ) + def on_slider_value(val): + from dash import ctx, no_update + + aio_id = ctx.outputs_grouping["id"]["aio_id"] + with lock: + fire_counts[aio_id] = fire_counts.get(aio_id, 0) + 1 + return no_update + + dash_duo.start_server(app) + + # Add first slider, should fire exactly once for slider "1" + dash_duo.find_element("#add-btn").click() + wait.until(lambda: fire_counts.get("1", 0) >= 1, 5) + assert ( + fire_counts["1"] == 1 + ), f"Slider 1 fired {fire_counts['1']} times after being added" + + # Add second slider, should fire once for "2", not refire "1" + dash_duo.find_element("#add-btn").click() + wait.until(lambda: fire_counts.get("2", 0) >= 1, 5) + assert fire_counts.get("1", 0) == 1, ( + f"Slider 1 spuriously refired after adding slider 2: " + f"count={fire_counts.get('1', 0)}" + ) + assert ( + fire_counts["2"] == 1 + ), f"Slider 2 fired {fire_counts['2']} times after being added" + + # Add third slider, should fire once for "3", not refire "1" or "2" + dash_duo.find_element("#add-btn").click() + wait.until(lambda: fire_counts.get("3", 0) >= 1, 5) + assert fire_counts.get("1", 0) == 1, ( + f"Slider 1 spuriously refired after adding slider 3: " + f"count={fire_counts.get('1', 0)}" + ) + assert fire_counts.get("2", 0) == 1, ( + f"Slider 2 spuriously refired after adding slider 3: " + f"count={fire_counts.get('2', 0)}" + ) + assert ( + fire_counts["3"] == 1 + ), f"Slider 3 fired {fire_counts['3']} times after being added" + + +def test_cbwc013_patch_rebuild_match_initial_call_undefined_output_prop(dash_duo): + """Ensure that initial callbacks are running when they're meant to + + When a Patch operation replaces an entire children list of a container with + fresh component instances that reuse the same MATCH ids as the prior + occupants, and the relevant output prop is undefined on both old and new + sides (a dcc.Slider with no explicit `value`), the per MATCH initial + callback should only fire once for each newly mounted slot + + Bug behavior: + Suppression that goes by "was this id already on the page" or by + comparing old and new prop values considers each rebuilt slider + unchanged, both sides are undefined, so its initial call is dropped + from the queue and never fires. The Python fire_counts dict therefore + stays at the pre rebuild value + + Expected behavior (after fix): + Every rebuilt slot fires the initial MATCH callback exactly once, + regardless of whether its output prop is defined: the patch created + these instances, so they are new whatever ids they reuse + + The MATCH callback used here outputs to `slider.value` (undefined on both + sides) and is keyed by an Input on `slider.min`. The input value doesn't + change across the rebuild, so the *only* path by which this callback can + fire for newly mounted instances is the layout level initial call path + (handleOneId -> outIdCallbacks) + """ + lock = threading.Lock() + fire_counts = {} # {idx: count} + N = 5 + + def make_slider_row(i): + return html.Div( + [ + dcc.Slider( + id={"type": "slider", "idx": i}, + min=0, + max=10, + step=0.1, + # NOTE: no `value` set, undefined in layout on both sides + ), + ] + ) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Rebuild", id="rebuild-btn", n_clicks=0), + html.Div( + id="container", + children=[make_slider_row(i) for i in range(N)], + ), + ] + ) + + @app.callback( + Output("container", "children"), + Input("rebuild-btn", "n_clicks"), + prevent_initial_call=True, + ) + def rebuild(_n): + # Children list replacement via Patch: clear + append rebuilds the + # entire list with fresh component instances. The MATCH ids match the + # prior occupants exactly, and so do the props, so nothing in the + # resulting layout tells the new instances apart from the ones they + # replaced, only the patch operations themselves know they are new + p = Patch() + p.clear() + for i in range(N): + p.append(make_slider_row(i)) + return p + + @app.callback( + Output({"type": "slider", "idx": MATCH}, "value"), + Input({"type": "slider", "idx": MATCH}, "min"), + prevent_initial_call=False, + ) + def on_slider_mount(_min_val): + from dash import ctx, no_update + + idx = ctx.outputs_grouping["id"]["idx"] + with lock: + fire_counts[idx] = fire_counts.get(idx, 0) + 1 + # Don't actually update `value`, keep it undefined on both sides, so + # that suppressing the initial call by comparing the old and new value + # would drop it for every rebuilt slider + return no_update + + dash_duo.start_server(app) + + # Initial mount: every slider should fire its MATCH callback exactly once + for i in range(N): + wait.until(lambda i=i: fire_counts.get(i, 0) >= 1, 5) + initial = dict(fire_counts) + for i in range(N): + assert ( + initial[i] == 1 + ), f"Slider {i} fired {initial[i]} times on initial mount (expected 1)" + + # Patch rebuilt the children list. Every slot is freshly mounted, so each + # MATCH callback should fire exactly once more + dash_duo.find_element("#rebuild-btn").click() + + # Wait long enough for any queued/dispatched callbacks to complete + import time + + time.sleep(3) + + failures = [] + for i in range(N): + expected = initial[i] + 1 + actual = fire_counts.get(i, 0) + if actual != expected: + failures.append(f" slot idx={i}: expected {expected} fires, got {actual}") + + assert not failures, ( + "After Patch driven children list rebuild, the following MATCH " + "callbacks did not refire as expected (initial call suppression bug):\n" + + "\n".join(failures) + + "\n\nThe renderer must only suppress the initial call for components " + "the Patch carried over, which it takes from the patch operations " + "themselves. Components rebuilt with a reused id are new instances and " + "must get their initial call, even when their output prop is undefined " + "on both sides" + ) + + assert dash_duo.get_logs() == [] + + +def test_cbwc014_patch_nested_change_does_not_refire_container_callbacks(dash_duo): + """A Patch that changes something deep inside a container + must not rerun the initial callbacks of the containers around it + + Patch operations are applied with ramda's assocPath, which rebuilds every + object between the patched prop and the value that changed. Those + containers are not new, the patch only carried them over, so their + initial callbacks must stay suppressed. Reference identity cannot tell + them apart from a component the patch created, since assocPath gives them + a brand new props object either way. The renderer instead records what + each patch operation did while applying it + + The same Patch also appends a genuinely new component, whose initial + callback must still fire, proving the suppression is not too broad + """ + lock = threading.Lock() + fire_counts = {} + + app = Dash(__name__, suppress_callback_exceptions=True) + app.layout = html.Div( + [ + html.Button("Patch", id="patch-btn", n_clicks=0), + # Never changes, so the counting callbacks below can only fire from + # the layout level initial call path + html.Div("static", id="static"), + html.Div( + [ + html.Div( + [dcc.Input(id="deep-input", value="initial")], + id="patched-group", + ), + html.Div(id="untouched-group"), + ], + id="container", + ), + html.Div("0", id="done"), + ] + ) + + def count_fire(name): + with lock: + fire_counts[name] = fire_counts.get(name, 0) + 1 + return f"fired-{name}" + + @app.callback( + Output("container", "children"), + Output("done", "children"), + Input("patch-btn", "n_clicks"), + prevent_initial_call=True, + ) + def do_patch(n): + p = Patch() + # Deep change: only `deep-input.value` is written, but assocPath has to + # rebuild the children list and `patched-group` on the way there + p[0]["props"]["children"][0]["props"]["value"] = f"patched-{n}" + # ... and one genuinely new component + p.append(html.Div(id="new-group")) + return p, str(n) + + @app.callback(Output("patched-group", "className"), Input("static", "children")) + def on_patched_group(_): + return count_fire("patched-group") + + @app.callback(Output("untouched-group", "className"), Input("static", "children")) + def on_untouched_group(_): + return count_fire("untouched-group") + + @app.callback(Output("new-group", "className"), Input("static", "children")) + def on_new_group(_): + return count_fire("new-group") + + dash_duo.start_server(app) + + # Both containers get their initial call once, on page load. `new-group` + # does not exist yet, so its callback cannot run + wait.until(lambda: fire_counts.get("patched-group", 0) >= 1, 5) + wait.until(lambda: fire_counts.get("untouched-group", 0) >= 1, 5) + assert fire_counts.get("new-group", 0) == 0 + + dash_duo.find_element("#patch-btn").click() + dash_duo.wait_for_text_to_equal("#done", "1") + + # The new component's initial call and any spurious refire of the existing + # ones are queued together, so waiting for it is a sync point for both + wait.until(lambda: fire_counts.get("new-group", 0) >= 1, 5) + wait.until( + lambda: dash_duo.find_element("#deep-input").get_attribute("value") + == "patched-1", + 5, + ) + + assert fire_counts["patched-group"] == 1, ( + "The container rebuilt by assocPath on the way to the patched value " + f"refired its initial callback: count={fire_counts['patched-group']}" + ) + assert fire_counts["untouched-group"] == 1, ( + "An untouched sibling container refired its initial callback: " + f"count={fire_counts['untouched-group']}" + ) + assert ( + fire_counts["new-group"] == 1 + ), f"The appended component fired {fire_counts['new-group']} times (expected 1)" + + assert dash_duo.get_logs() == [] diff --git a/tests/integration/renderer/test_persistence.py b/tests/integration/renderer/test_persistence.py index fd3ce98ee0..bd1491785c 100644 --- a/tests/integration/renderer/test_persistence.py +++ b/tests/integration/renderer/test_persistence.py @@ -5,8 +5,21 @@ from selenium.webdriver.common.keys import Keys +from dash.testing import wait + import dash -from dash import Dash, Input, Output, State, MATCH, dcc, html, dash_table as dt +from dash import ( + Dash, + Input, + Output, + State, + MATCH, + ALL, + Patch, + dcc, + html, + dash_table as dt, +) from dash_test_components import MyPersistedComponent from dash_test_components import MyPersistedComponentNested @@ -602,3 +615,254 @@ def update_output_2(value): # Check that persisted values are restored dash_duo.wait_for_text_to_equal("#output-1", "Output 1: changed1") dash_duo.wait_for_text_to_equal("#output-2", "Output 2: changed2") + + +def test_rdps015_patch_preserves_persistence(dash_duo): + """Test carry over check in applyPersistence/persistenceMods + + When Patch() appends a new component to a list of persisted components, + applyPersistence must not clear the localStorage entries for preexisting + components + + The issue being checked: parsePatchProps resolves a Patch against the current Redux state, + producing a full children array where preexisting components carry their + current Redux values (value="edited" after user interaction). When + applyPersistence then recurses all children, persistenceMods calls modProp + on every component, including preexisting ones. modProp compares + originalVal ("initial", stored in localStorage) against props.value ("edited", + from Redux), sees a mismatch, and clears the localStorage entry + + The fix uses the patch operations record, created during application. + persistenceMods only runs modProp on the components the patch actually + created or wrote a prop on, and leaves the ones it carried over alone + """ + + def make_input(index): + return html.Div( + dcc.Input( + id={"type": "persist-input", "index": index}, + value="initial", + persistence=True, + persistence_type="local", + className="persist-input", + ) + ) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Add", id="add-btn", n_clicks=0), + html.Div([make_input(0), make_input(1)], id="container"), + html.Div(id="display"), + ] + ) + + @app.callback( + Output("container", "children"), + Input("add-btn", "n_clicks"), + prevent_initial_call=True, + ) + def add_input(n): + p = Patch() + p.append(make_input(n + 1)) + return p + + @app.callback( + Output("display", "children"), + Input({"type": "persist-input", "index": ALL}, "value"), + ) + def show_all(values): + return "|".join(str(v) for v in values) + + dash_duo.start_server(app) + + dash_duo.wait_for_text_to_equal("#display", "initial|initial") + + # Edit the first input so its value is persisted to localStorage as "edited" + first_input = dash_duo.find_elements(".persist-input input")[0] + first_input.send_keys(Keys.CONTROL + "a") + first_input.send_keys("edited") + dash_duo.wait_for_text_to_equal("#display", "edited|initial") + + # Add a new component via Patch, this is the operation that previously + # caused applyPersistence to clear the localStorage entry for input 0 + dash_duo.find_element("#add-btn").click() + dash_duo.wait_for_text_to_equal("#display", "edited|initial|initial") + + # Reload the page. The Patch was ephemeral (serverside layout only has the + # original two inputs). If localStorage was cleared by the Patch update, + # input 0 will revert to "initial". If the fix is working, "edited" is + # restored from localStorage + dash_duo.wait_for_page() + # If localStorage was cleared by the Patch update, input 0 reverts to "initial" + # The fix preserves the entry, so "edited" is restored from localStorage + dash_duo.wait_for_text_to_equal("#display", "edited|initial") + + +def test_rdps016_patch_output_does_not_disable_persistence_of_sibling(dash_duo): + """Confirm that patchyness is tracked per output, not per callback + + A multioutput callback can return a Patch for one output and a full + replacement for another. Only the patched output carries preexisting + components over from Redux (with their user edited values), so only that + output may skip applyPersistence. The fully replaced output comes back with + fresh server defaults, so persisted user edits must still be restored + + Before the fix, a single "this result contained a Patch" flag was attached + to the whole callback result and applied to every output, so components in + the replaced output were treated as preexisting and their persistence + restore was silently skipped, the user's edit was overwritten by the + server default + """ + + def make_patched_input(index): + return dcc.Input( + id={"type": "patched-input", "index": index}, + value="initial", + persistence=True, + persistence_type="local", + ) + + def make_replaced_input(): + return dcc.Input( + id="replaced-input", + value="server-default", + persistence=True, + persistence_type="local", + ) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Go", id="go-btn", n_clicks=0), + html.Div([make_patched_input(0)], id="patched-container"), + html.Div([make_replaced_input()], id="replaced-container"), + html.Div(id="display"), + ] + ) + + @app.callback( + Output("patched-container", "children"), + Output("replaced-container", "children"), + Input("go-btn", "n_clicks"), + prevent_initial_call=True, + ) + def go(n): + p = Patch() + p.append(make_patched_input(n)) + # Full replacement, reusing the same id: a fresh instance carrying the + # server default, whose persisted value must be restored + return p, [make_replaced_input()] + + @app.callback(Output("display", "children"), Input("replaced-input", "value")) + def show(value): + return f"replaced={value}" + + dash_duo.start_server(app) + + dash_duo.wait_for_text_to_equal("#display", "replaced=server-default") + + # Edit the input so "edited" is persisted to localStorage + replaced_input = dash_duo.find_element("#replaced-input") + replaced_input.send_keys(Keys.CONTROL + "a") + replaced_input.send_keys("edited") + dash_duo.wait_for_text_to_equal("#display", "replaced=edited") + + dash_duo.find_element("#go-btn").click() + + # Both outputs are applied in the same render pass, so once the patched + # output shows two inputs (each dcc.Input renders wrapped in its own + # container div, so they aren't adjacent siblings) the replaced output + # has been applied too + wait.until(lambda: len(dash_duo.find_elements("#patched-container input")) == 2, 10) + + assert ( + dash_duo.find_element("#replaced-input").get_attribute("value") == "edited" + ), ( + "The persisted value was not restored for the fully replaced output: " + "patchyness must be tracked per output, not per callback." + ) + dash_duo.wait_for_text_to_equal("#display", "replaced=edited") + + assert dash_duo.get_logs() == [] + + +def test_rdps017_patch_rebuild_same_id_restores_persistence(dash_duo): + """Confirm that carry over detection goes by what the patch created, + not by ids were already on the page + + A Patch that rebuilds a children list by clearing it and reappending + components that reuse the same ids produces genuinely new component + instances, they are not carried over from Redux, even though their + ids were seen before the update + """ + + def make_input(value="initial"): + return dcc.Input( + id="rebuilt-input", + value=value, + persistence=True, + persistence_type="local", + ) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Button("Rebuild", id="rebuild-btn", n_clicks=0), + html.Div([make_input()], id="container"), + html.Div(id="counter", children="0"), + html.Div(id="display"), + ] + ) + + @app.callback( + Output("container", "children"), + Output("counter", "children"), + Input("rebuild-btn", "n_clicks"), + prevent_initial_call=True, + ) + def rebuild(n): + p = Patch() + p.clear() + # Reuses the same id as before with the same server default value, but + # this is a genuinely new component instance, not carried over from + # Redux state. Persistence only restores an edit when the component's + # current default value matches the one recorded when the edit was + # made (see modProp), so the default must stay "initial" here + p.append(make_input()) + # A plain, non persisted output that changes on every click, so tests + # can wait for the rebuild to have been applied without depending on + # the (possibly unchanged) persisted value itself + return p, str(n) + + @app.callback(Output("display", "children"), Input("rebuilt-input", "value")) + def show(value): + return f"value={value}" + + dash_duo.start_server(app) + + dash_duo.wait_for_text_to_equal("#display", "value=initial") + + # Edit the input so "edited" is persisted to localStorage + # Use Ctrl+A + type because .clear() doesn't work reliably + # with react inputs + rebuilt_input = dash_duo.find_element("#rebuilt-input") + rebuilt_input.send_keys(Keys.CONTROL + "a") + rebuilt_input.send_keys("edited") + dash_duo.wait_for_text_to_equal("#display", "value=edited") + + dash_duo.find_element("#rebuild-btn").click() + + # Wait for the rebuild to be applied (the counter changes regardless of + # what value the rebuilt input ends up with), then check whether + # persistence restored the user's edit on top of the rebuilt component + dash_duo.wait_for_text_to_equal("#counter", "1") + + assert dash_duo.find_element("#rebuilt-input").get_attribute("value") == "edited", ( + "The persisted value was not restored for a component rebuilt with a " + "reused id: a component the patch inserted is a new instance, whatever " + "id it reuses, so persistence must run on it." + ) + dash_duo.wait_for_text_to_equal("#display", "value=edited") + + assert dash_duo.get_logs() == []