Skip to content

Fix dcc.Patch() disturbing pre-existing components (re-run callbacks + wiped persistence) - #3938

Open
Aaron-Wrote-This wants to merge 3 commits into
plotly:devfrom
Aaron-Wrote-This:bugfix/patch_reruns_initial_cbs_and_wipes_persisted_vals
Open

Fix dcc.Patch() disturbing pre-existing components (re-run callbacks + wiped persistence)#3938
Aaron-Wrote-This wants to merge 3 commits into
plotly:devfrom
Aaron-Wrote-This:bugfix/patch_reruns_initial_cbs_and_wipes_persisted_vals

Conversation

@Aaron-Wrote-This

Copy link
Copy Markdown
Contributor

Fixes #3681 and #3937

Problem

When a dcc.Patch() added or rebuilt components inside a container, two separate bugs trigger,
effecting components which already existed before the Patch:

  1. Initial callbacks refire for every matching MATCH/ALL output, so appending one new
    pattern matching id'd dcc.Slider reruns the initial callback for every preexisting slider
  2. Persisted user edits are wiped, so edits to persistence enabled components are cleared
    because the patch added a new sibling

I tracked both items back to the way Patch resolution distinguishes components carried over from the
pre-Patch layout from freshly built ones

Fix

  1. Snapshot the paths table before a Patch is resolved

  2. Callbacks can suppress the initial call for carried-over components, by detecting props
    reference identity (child.props === oldPropsRef). ramda's assocPath is structurally immutable,
    so untouched nodes keep their exact props reference while rebuilt/replaced node gets a new one

  3. applyPersistence now skips components that already existed pre-Patch, instead of re-running
    modProp and misreading the carried over value as a "server override" that clears the stored edit

Contributor Checklist

I've been doing a lot of work with dynamic UIs and creating extensible elements. I am loving the patch system, but I found some significant drawbacks to the both in performance and usability.

At first, I thought the fix for the initial callbacks was simple and just checking the ID more carefully would be enough, but it ended up being requiring adding an 'audit trail' for patches, which also ended up fixing the persistence issues.

Javascript isn't my 'first language', so let me know if there are any things I'm doing awkwardly, or best practices I'm missing. Happy to fix those up.
Or I understand if this is too complicated and not exactly needed. I looked for simpler options, but I couldn't find a way to differentiate patches and understand their impact in any other way, but it would be great if one existed and I just didn't see it.

  • I have broken down my PR scope into the following TODO tasks
    • Record patch operations
    • Fix patch running against existing components with matching pattern matching IDs
    • Fix persisted values being wiped from existing components during a patch
  • I have run the tests locally and they passed
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

* As each Patch() operation is applied, record which component ids are
  created and which props are written
* Store one PatchAnalysis per output that returned a Patch, keyed by the
  output's id, on the callback result's patchedOutputs map
* To be used to for tracking what the Patch actually created vs elements
  carried over unchanged
* Before, when a Patch added or rebuilt a container's children list,
  getUnfilteredLayoutCallbacks re-fired the initial call for every
  MATCH/ALL callback bound to a component, even if it already existed
  before the Patch
* Now, initial calls are gated on Patch operations actually change
  * patchAnalysis.ts records each patch operation and which component
    ids are being created
  * handleOneId suppresses the initial call only for ids the patch did
    not create. This also correctly fires for a component rebuilt with
    an id that was already in use, even when its new defaults happen to
    coincide with the prior occupant's values
* Add regression tests test_wildcards: 11, 12, 13
* applyPersistence ran unconditionally on every component reachable from
  a Patch result, including ones the Patch carried over unchanged
  For a persisted component, that meant the just-carried-over value was
  treated as a fresh server default and overwrote the user's stored edit
* Now skip persistence restoration, for components the patch did not
  create, by having persistenceMods consults the PatchAnalysis via
  isUntouchedByPatch, so a component genuinely carried over keeps its
  persisted value, while one rebuilt with a reused id still gets its
  persisted value restored

* * Add regression tests to test_persistence: 15, 16, 17
@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] dash.Patch() reruns inital callback for all matching elements

1 participant