diff --git a/dash/dash-renderer/src/TreeContainer.js b/dash/dash-renderer/src/TreeContainer.js index 3c2c465ba1..eae6ac5e95 100644 --- a/dash/dash-renderer/src/TreeContainer.js +++ b/dash/dash-renderer/src/TreeContainer.js @@ -19,7 +19,7 @@ import { } from 'ramda'; import {notifyObservers, updateProps} from './actions'; import isSimpleComponent from './isSimpleComponent'; -import {recordUiEdit} from './persistence'; +import {recordEdit} from './persistence'; import ComponentErrorBoundary from './components/error/ComponentErrorBoundary.react'; import checkPropTypes from './checkPropTypes'; import {getWatchedKeys, stringifyId} from './actions/dependencies'; @@ -136,7 +136,7 @@ class BaseTreeContainer extends Component { // setProps here is triggered by the UI - record these changes // for persistence - recordUiEdit(_dashprivate_layout, newProps, _dashprivate_dispatch); + recordEdit(_dashprivate_layout, newProps, _dashprivate_dispatch); // Only dispatch changes to Dash if a watched prop changed if (watchedKeys.length) { diff --git a/dash/dash-renderer/src/observers/executedCallbacks.ts b/dash/dash-renderer/src/observers/executedCallbacks.ts index 8062bbf194..abcd714c81 100644 --- a/dash/dash-renderer/src/observers/executedCallbacks.ts +++ b/dash/dash-renderer/src/observers/executedCallbacks.ts @@ -36,7 +36,7 @@ import {ICallback, IStoredCallback} from '../types/callbacks'; import {updateProps, setPaths, handleAsyncError} from '../actions'; import {getPath, computePaths} from '../actions/paths'; -import {applyPersistence, prunePersistence} from '../persistence'; +import {applyPersistence, prunePersistence, recordEdit} from '../persistence'; import {IStoreObserverDefinition} from '../StoreObserver'; const observer: IStoreObserverDefinition = { @@ -65,6 +65,9 @@ const observer: IStoreObserverDefinition = { // those components have props to update to persist user edits. const {props} = applyPersistence({props: updatedProps}, dispatch); + // Save props to storage if persistance is active. + recordEdit(path(itempath, layout), updatedProps, dispatch); + dispatch( updateProps({ itempath, diff --git a/dash/dash-renderer/src/persistence.js b/dash/dash-renderer/src/persistence.js index b7bc2719ba..72a955d7e7 100644 --- a/dash/dash-renderer/src/persistence.js +++ b/dash/dash-renderer/src/persistence.js @@ -306,7 +306,7 @@ const getProps = layout => { }; }; -export function recordUiEdit(layout, newProps, dispatch) { +export function recordEdit(layout, newProps, dispatch) { const { canPersist, id, @@ -316,7 +316,18 @@ export function recordUiEdit(layout, newProps, dispatch) { persisted_props, persistence_type } = getProps(layout); - if (!canPersist || !persistence) { + + const getFinal = (propName, prevVal) => + propName in newProps ? newProps[propName] : prevVal; + const finalPersistence = getFinal('persistence', persistence); + const finalPersistenceType = getFinal('persistence_type', persistence_type); + + if ( + !canPersist || + !finalPersistence || + finalPersistence !== persistence || + finalPersistenceType !== persistence_type + ) { return; } @@ -336,6 +347,10 @@ export function recordUiEdit(layout, newProps, dispatch) { if (originalVal !== newVal) { if (storage.hasItem(valsKey)) { originalVal = storage.getItem(valsKey)[1]; + if (newVal === originalVal) { + storage.removeItem(valsKey); + return; + } } const vals = originalVal === undefined @@ -517,28 +532,6 @@ export function prunePersistence(layout, newProps, dispatch) { filter(notInNewProps, finalPersistedProps) ); } - - // now the main point - clear any edit of a prop that changed - // note that this is independent of the new prop value. - const transforms = element.persistenceTransforms || {}; - for (const propName in newProps) { - const propTransforms = transforms[propName]; - if (propTransforms) { - for (const propPart in propTransforms) { - finalStorage.removeItem( - getValsKey( - id, - `${propName}.${propPart}`, - finalPersistence - ) - ); - } - } else { - finalStorage.removeItem( - getValsKey(id, propName, finalPersistence) - ); - } - } } return persistenceChanged ? mergeRight(newProps, update) : newProps; } diff --git a/dash/dash-renderer/tests/persistence.test.js b/dash/dash-renderer/tests/persistence.test.js index 0a9c60924c..2c85e4be1b 100644 --- a/dash/dash-renderer/tests/persistence.test.js +++ b/dash/dash-renderer/tests/persistence.test.js @@ -1,6 +1,6 @@ import {expect} from 'chai'; import {afterEach, beforeEach, describe, it} from 'mocha'; -import {recordUiEdit, stores, storePrefix} from '../src/persistence'; +import {recordEdit, stores, storePrefix} from '../src/persistence'; const longString = pow => { let s = 's'; for (let i = 0; i < pow; i++) { @@ -99,7 +99,7 @@ describe('storage fallbacks and equivalence', () => { const layout = layoutA(storeType); it(`empty ${storeName} works`, () => { - recordUiEdit(layout, {p1: propVal}, _dispatch); + recordEdit(layout, {p1: propVal}, _dispatch); expect(dispatchCalls).to.deep.equal([]); expect(store.getItem(`${storePrefix}a.p1.true`)).to.equal( `[${propStr}]` @@ -109,7 +109,7 @@ describe('storage fallbacks and equivalence', () => { it(`${storeName} full from persistence works with warnings`, () => { fillStorage(store, `${storePrefix}x.x`); - recordUiEdit(layout, {p1: propVal}, _dispatch); + recordEdit(layout, {p1: propVal}, _dispatch); expect(dispatchCalls).to.deep.equal([ `${storeName} init first try failed; clearing and retrying`, `${storeName} init set/get succeeded after clearing!` @@ -125,7 +125,7 @@ describe('storage fallbacks and equivalence', () => { it(`${storeName} full from other stuff falls back on memory`, () => { fillStorage(store, 'not_ours'); - recordUiEdit(layout, {p1: propVal}, _dispatch); + recordEdit(layout, {p1: propVal}, _dispatch); expect(dispatchCalls).to.deep.deep.equal([ `${storeName} init first try failed; clearing and retrying`, `${storeName} init still failed, falling back to memory` @@ -139,11 +139,11 @@ describe('storage fallbacks and equivalence', () => { // Maybe not ideal long-term behavior, but this is what happens // initialize and ensure the store is happy - recordUiEdit(layout, {p1: propVal}, _dispatch); + recordEdit(layout, {p1: propVal}, _dispatch); expect(dispatchCalls).to.deep.deep.equal([]); // now flood it. - recordUiEdit(layout, {p1: longString(26)}, _dispatch); + recordEdit(layout, {p1: longString(26)}, _dispatch); expect(dispatchCalls).to.deep.equal([ `a.p1.true failed to save in ${storeName}. Persisted props may be lost.` ]); @@ -156,7 +156,7 @@ describe('storage fallbacks and equivalence', () => { it(`${storeType} primitives in/out match`, () => { // ensure storage is instantiated - recordUiEdit(layout, {p1: propVal}, _dispatch); + recordEdit(layout, {p1: propVal}, _dispatch); const store = stores[storeType]; [ 0, @@ -177,7 +177,7 @@ describe('storage fallbacks and equivalence', () => { }); it(`${storeType} arrays and objects in/out are clones`, () => { - recordUiEdit(layout, {p1: propVal}, _dispatch); + recordEdit(layout, {p1: propVal}, _dispatch); const store = stores[storeType]; [[1, 2, 3], {a: 1, b: 2}].forEach(val => { diff --git a/tests/integration/renderer/test_persistence.py b/tests/integration/renderer/test_persistence.py index 701d4886e4..801efb0bd0 100644 --- a/tests/integration/renderer/test_persistence.py +++ b/tests/integration/renderer/test_persistence.py @@ -558,3 +558,64 @@ def update_container2(n_clicks): # persistenceTransforms should return upper case strings dash_duo.wait_for_text_to_equal("#component-propName", "ALPACA") dash_duo.wait_for_text_to_equal("#component-propPart", "ARTICHOKE") + + +def test_rdps014_save_callback_persistence(dash_duo): + app = dash.Dash(__name__) + + def make_input(persistence): + return dcc.Input(id="persisted", value="a", persistence=persistence) + + app.layout = html.Div( + [ + dcc.Input(id="persistence-val", value=""), + dcc.Input(id="persistence-key", value=""), + html.Div(make_input(""), id="persisted-container"), + html.Div(id="out"), + ] + ) + + # this is not a good way to set persistence, as it doesn't allow you to + # get the right initial value. Much better is to update the whole component + # as we do in the previous test case... but it shouldn't break this way. + @app.callback( + Output("persisted", "persistence"), [Input("persistence-key", "value")] + ) + def set_persistence(val): + return val + + @app.callback(Output("persisted", "value"), [Input("persistence-val", "value")]) + def set_value(val): + return val + + @app.callback(Output("out", "children"), [Input("persisted", "value")]) + def set_out(val): + return val + + dash_duo.start_server(app) + + dash_duo.wait_for_text_to_equal("#out", "") + + dash_duo.find_element("#persistence-key").send_keys("a") + time.sleep(0.2) + assert not dash_duo.get_logs() + dash_duo.wait_for_text_to_equal("#out", "") + dash_duo.find_element("#persistence-val").send_keys("alpaca") + dash_duo.wait_for_text_to_equal("#out", "alpaca") + + dash_duo.find_element("#persistence-key").send_keys("b") + dash_duo.wait_for_text_to_equal("#out", "") + dash_duo.clear_input("#persistence-val") + dash_duo.find_element("#persistence-val").send_keys("artichoke") + dash_duo.wait_for_text_to_equal("#out", "artichoke") + + # no persistence, still goes back to original value + dash_duo.clear_input("#persistence-key") + dash_duo.wait_for_text_to_equal("#out", "") + + # apricot and artichoke saved + dash_duo.find_element("#persistence-key").send_keys("a") + dash_duo.wait_for_text_to_equal("#out", "alpaca") + dash_duo.find_element("#persistence-key").send_keys("b") + assert not dash_duo.get_logs() + dash_duo.wait_for_text_to_equal("#out", "artichoke")