diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cdfc8931b..1c84c854d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - [#1078](https://github.com/plotly/dash/pull/1078) Permit usage of arbitrary file extensions for assets within component libraries ### Fixed +- [#1220](https://github.com/plotly/dash/pull/1220) Fixes [#1216](https://github.com/plotly/dash/issues/1216), a set of related issues about pattern-matching callbacks with `ALL` wildcards in their `Output` which would fail if no components matched the pattern. - [#1212](https://github.com/plotly/dash/pull/1212) Fixes [#1200](https://github.com/plotly/dash/issues/1200) - prior to Dash 1.11, if none of the inputs to a callback were on the page, it was not an error. This was, and is now again, treated as though the callback raised PreventUpdate. The one exception to this is with pattern-matching callbacks, when every Input uses a multi-value wildcard (ALL or ALLSMALLER), and every Output is on the page. In that case the callback fires as usual. - [#1201](https://github.com/plotly/dash/pull/1201) Fixes [#1193](https://github.com/plotly/dash/issues/1193) - prior to Dash 1.11, you could use `flask.has_request_context() == False` inside an `app.layout` function to provide a special layout containing all IDs for validation purposes in a multi-page app. Dash 1.11 broke this when we moved most of this validation into the renderer. This change makes it work again. diff --git a/dash-renderer/src/actions/dependencies.js b/dash-renderer/src/actions/dependencies.js index 8ae7a7faff..59f398b010 100644 --- a/dash-renderer/src/actions/dependencies.js +++ b/dash-renderer/src/actions/dependencies.js @@ -10,6 +10,7 @@ import { dissoc, equals, evolve, + findIndex, flatten, forEachObjIndexed, includes, @@ -396,12 +397,12 @@ function findInOutOverlap(outputs, inputs, head, dispatchError) { } function findMismatchedWildcards(outputs, inputs, state, head, dispatchError) { - const {anyKeys: out0AnyKeys} = findWildcardKeys(outputs[0].id); - outputs.forEach((out, outi) => { - if (outi && !equals(findWildcardKeys(out.id).anyKeys, out0AnyKeys)) { + const {matchKeys: out0MatchKeys} = findWildcardKeys(outputs[0].id); + outputs.forEach((out, i) => { + if (i && !equals(findWildcardKeys(out.id).matchKeys, out0MatchKeys)) { dispatchError('Mismatched `MATCH` wildcards across `Output`s', [ head, - `Output ${outi} (${combineIdAndProp(out)})`, + `Output ${i} (${combineIdAndProp(out)})`, 'does not have MATCH wildcards on the same keys as', `Output 0 (${combineIdAndProp(outputs[0])}).`, 'MATCH wildcards must be on the same keys for all Outputs.', @@ -414,9 +415,9 @@ function findMismatchedWildcards(outputs, inputs, state, head, dispatchError) { [state, 'State'], ].forEach(([args, cls]) => { args.forEach((arg, i) => { - const {anyKeys, allsmallerKeys} = findWildcardKeys(arg.id); - const allWildcardKeys = anyKeys.concat(allsmallerKeys); - const diff = difference(allWildcardKeys, out0AnyKeys); + const {matchKeys, allsmallerKeys} = findWildcardKeys(arg.id); + const allWildcardKeys = matchKeys.concat(allsmallerKeys); + const diff = difference(allWildcardKeys, out0MatchKeys); if (diff.length) { diff.sort(); dispatchError('`Input` / `State` wildcards not in `Output`s', [ @@ -639,7 +640,7 @@ export function computeGraphs(dependencies, dispatchError) { * {[id]: {[prop]: [callback, ...]}} * where callbacks are the matching specs from the original * dependenciesRequest, but with outputs parsed to look like inputs, - * and a list anyKeys added if the outputs have MATCH wildcards. + * and a list matchKeys added if the outputs have MATCH wildcards. * For outputMap there should only ever be one callback per id/prop * but for inputMap there may be many. * @@ -785,9 +786,10 @@ export function computeGraphs(dependencies, dispatchError) { // Also collect MATCH keys in the output (all outputs must share these) // and ALL keys in the first output (need not be shared but we'll use // the first output for calculations) for later convenience. - const {anyKeys, hasALL} = findWildcardKeys(outputs[0].id); + const {matchKeys} = findWildcardKeys(outputs[0].id); + const firstSingleOutput = findIndex(o => !isMultiValued(o.id), outputs); const finalDependency = mergeRight( - {hasALL, anyKeys, outputs}, + {matchKeys, firstSingleOutput, outputs}, dependency ); @@ -820,23 +822,20 @@ export function computeGraphs(dependencies, dispatchError) { } function findWildcardKeys(id) { - const anyKeys = []; + const matchKeys = []; const allsmallerKeys = []; - let hasALL = false; if (typeof id === 'object') { forEachObjIndexed((val, key) => { if (val === MATCH) { - anyKeys.push(key); + matchKeys.push(key); } else if (val === ALLSMALLER) { allsmallerKeys.push(key); - } else if (val === ALL) { - hasALL = true; } }, id); - anyKeys.sort(); + matchKeys.sort(); allsmallerKeys.sort(); } - return {anyKeys, allsmallerKeys, hasALL}; + return {matchKeys, allsmallerKeys}; } /* @@ -1048,31 +1047,6 @@ function getCallbackByOutput(graphs, paths, id, prop) { return makeResolvedCallback(callback, resolve, anyVals); } -/* - * If there are ALL keys we need to reduce a set of outputs resolved - * from an input to one item per combination of MATCH values. - * That will give one result per callback invocation. - */ -function reduceALLOuts(outs, anyKeys, hasALL) { - if (!hasALL) { - return outs; - } - if (!anyKeys.length) { - // If there's ALL but no MATCH, there's only one invocation - // of the callback, so just base it off the first output. - return [outs[0]]; - } - const anySeen = {}; - return outs.filter(i => { - const matchKeys = JSON.stringify(props(anyKeys, i.id)); - if (!anySeen[matchKeys]) { - anySeen[matchKeys] = 1; - return true; - } - return false; - }); -} - function addResolvedFromOutputs(callback, outPattern, outs, matches) { const out0Keys = Object.keys(outPattern.id).sort(); const out0PatternVals = props(out0Keys, outPattern.id); @@ -1088,6 +1062,51 @@ function addResolvedFromOutputs(callback, outPattern, outs, matches) { }); } +function addAllResolvedFromOutputs(resolve, paths, matches) { + return callback => { + const {matchKeys, firstSingleOutput, outputs} = callback; + if (matchKeys.length) { + const singleOutPattern = outputs[firstSingleOutput]; + if (singleOutPattern) { + addResolvedFromOutputs( + callback, + singleOutPattern, + resolve(paths)(singleOutPattern), + matches + ); + } else { + /* + * If every output has ALL we need to reduce resolved set + * to one item per combination of MATCH values. + * That will give one result per callback invocation. + */ + const anySeen = {}; + outputs.forEach(outPattern => { + const outSet = resolve(paths)(outPattern).filter(i => { + const matchStr = JSON.stringify(props(matchKeys, i.id)); + if (!anySeen[matchStr]) { + anySeen[matchStr] = 1; + return true; + } + return false; + }); + addResolvedFromOutputs( + callback, + outPattern, + outSet, + matches + ); + }); + } + } else { + const cb = makeResolvedCallback(callback, resolve, ''); + if (flatten(cb.getOutputs(paths)).length) { + matches.push(cb); + } + } + }; +} + /* * For a given id and prop find all callbacks it's an input of. * @@ -1111,21 +1130,9 @@ export function getCallbacksByInput(graphs, paths, id, prop, changeType) { return []; } - const baseResolve = resolveDeps(); - callbacks.forEach(callback => { - const {anyKeys, hasALL} = callback; - if (anyKeys) { - const out0Pattern = callback.outputs[0]; - const out0Set = reduceALLOuts( - baseResolve(paths)(out0Pattern), - anyKeys, - hasALL - ); - addResolvedFromOutputs(callback, out0Pattern, out0Set, matches); - } else { - matches.push(makeResolvedCallback(callback, baseResolve, '')); - } - }); + callbacks.forEach( + addAllResolvedFromOutputs(resolveDeps(), paths, matches) + ); } else { // wildcard version const keys = Object.keys(id).sort(); @@ -1137,23 +1144,13 @@ export function getCallbacksByInput(graphs, paths, id, prop, changeType) { } patterns.forEach(pattern => { if (idMatch(keys, vals, pattern.values)) { - const resolve = resolveDeps(keys, vals, pattern.values); - pattern.callbacks.forEach(callback => { - const out0Pattern = callback.outputs[0]; - const {anyKeys, hasALL} = callback; - const out0Set = reduceALLOuts( - resolve(paths)(out0Pattern), - anyKeys, - hasALL - ); - - addResolvedFromOutputs( - callback, - out0Pattern, - out0Set, + pattern.callbacks.forEach( + addAllResolvedFromOutputs( + resolveDeps(keys, vals, pattern.values), + paths, matches - ); - }); + ) + ); } }); } diff --git a/tests/integration/callbacks/test_missing_outputs.py b/tests/integration/callbacks/test_missing_outputs.py new file mode 100644 index 0000000000..ce74ed8173 --- /dev/null +++ b/tests/integration/callbacks/test_missing_outputs.py @@ -0,0 +1,243 @@ +import pytest + +import dash_html_components as html +# import dash_core_components as dcc +import dash +from dash.dependencies import Input, Output, ALL, MATCH + +debugging = dict( + debug=True, use_reloader=False, use_debugger=True, dev_tools_hot_reload=False +) + + +@pytest.mark.parametrize("with_simple", (False, True)) +def test_cbmo001_all_output(with_simple, dash_duo): + app = dash.Dash(__name__) + + app.layout = html.Div(children=[ + html.Button("items", id="items"), + html.Button("values", id="values"), + html.Div(id="content"), + html.Div("Output init", id="output"), + ]) + + @app.callback(Output("content", "children"), [Input("items", "n_clicks")]) + def content(n1): + return [html.Div(id={"i": i}) for i in range((n1 or 0) % 4)] + + # these two variants have identical results, but the internal behavior + # is different when you combine the callbacks. + if with_simple: + @app.callback( + [Output({"i": ALL}, "children"), Output("output", "children")], + [Input("values", "n_clicks"), Input({"i": ALL}, "id")] + ) + def content_and_output(n2, content_ids): + # this variant *does* get called with empty ALL, because of the + # second Output + # TODO: however it doesn't get *triggered* by the ALL emptying, + # should it? for now, added content_ids to get proper triggering. + n1 = len(content_ids) + content = [n2 or 0] * n1 + return content, sum(content) + + else: + @app.callback(Output({"i": ALL}, "children"), [Input("values", "n_clicks")]) + def content_inner(n2): + # this variant does NOT get called with empty ALL + # the second callback handles outputting 0 in that case. + # if it were to be called throw an error so we'll see it in get_logs + n1 = len(dash.callback_context.outputs_list) + if not n1: + raise ValueError("should not be called with no outputs!") + return [n2 or 0] * n1 + + @app.callback(Output("output", "children"), [Input({"i": ALL}, "children")]) + def out2(contents): + return sum(contents) + + dash_duo.start_server(app) + + dash_duo.wait_for_text_to_equal("#content", "") + dash_duo.wait_for_text_to_equal("#output", "0") + + actions = [ + ["#values", "", "0"], + ["#items", "1", "1"], + ["#values", "2", "2"], + ["#items", "2\n2", "4"], + ["#values", "3\n3", "6"], + ["#items", "3\n3\n3", "9"], + ["#values", "4\n4\n4", "12"], + ["#items", "", "0"], + ["#values", "", "0"], + ["#items", "5", "5"] + ] + for selector, content, output in actions: + dash_duo.find_element(selector).click() + dash_duo.wait_for_text_to_equal("#content", content) + dash_duo.wait_for_text_to_equal("#output", output) + + assert not dash_duo.get_logs() + + +@pytest.mark.parametrize("with_simple", (False, True)) +def test_cbmo002_all_and_match_output(with_simple, dash_duo): + app = dash.Dash(__name__) + + app.layout = html.Div(children=[ + html.Button("items", id="items"), + html.Button("values", id="values"), + html.Div(id="content"), + ]) + + @app.callback(Output("content", "children"), [Input("items", "n_clicks")]) + def content(n1): + return [ + html.Div([ + html.Div( + [html.Div(id={"i": i, "j": j}) for i in range(((n1 or 0) + j) % 4)], + className="content{}".format(j) + ), + html.Div(id={"j": j}, className="output{}".format(j)), + html.Hr(), + ]) + for j in range(4) + ] + + # these two variants have identical results, but the internal behavior + # is different when you combine the callbacks. + if with_simple: + @app.callback( + [Output({"i": ALL, "j": MATCH}, "children"), Output({"j": MATCH}, "children")], + [Input("values", "n_clicks"), Input({"i": ALL, "j": MATCH}, "id")] + ) + def content_and_output(n2, content_ids): + # this variant *does* get called with empty ALL, because of the + # second Output + # TODO: however it doesn't get *triggered* by the ALL emptying, + # should it? for now, added content_ids to get proper triggering. + n1 = len(content_ids) + content = [n2 or 0] * n1 + return content, sum(content) + + else: + @app.callback( + Output({"i": ALL, "j": MATCH}, "children"), + [Input("values", "n_clicks")] + ) + def content_inner(n2): + # this variant does NOT get called with empty ALL + # the second callback handles outputting 0 in that case. + # if it were to be called throw an error so we'll see it in get_logs + n1 = len(dash.callback_context.outputs_list) + if not n1: + raise ValueError("should not be called with no outputs!") + return [n2 or 0] * n1 + + @app.callback( + Output({"j": MATCH}, "children"), + [Input({"i": ALL, "j": MATCH}, "children")] + ) + def out2(contents): + return sum(contents) + + dash_duo.start_server(app, **debugging) + + dash_duo.wait_for_text_to_equal(".content0", "") + dash_duo.wait_for_text_to_equal(".output0", "0") + + actions = [ + ["#values", [["", "0"], ["1", "1"], ["1\n1", "2"], ["1\n1\n1", "3"]]], + ["#items", [["1", "1"], ["1\n1", "2"], ["1\n1\n1", "3"], ["", "0"]]], + ["#values", [["2", "2"], ["2\n2", "4"], ["2\n2\n2", "6"], ["", "0"]]], + ["#items", [["2\n2", "4"], ["2\n2\n2", "6"], ["", "0"], ["2", "2"]]], + ["#values", [["3\n3", "6"], ["3\n3\n3", "9"], ["", "0"], ["3", "3"]]], + ["#items", [["3\n3\n3", "9"], ["", "0"], ["3", "3"], ["3\n3", "6"]]], + ["#values", [["4\n4\n4", "12"], ["", "0"], ["4", "4"], ["4\n4", "8"]]], + ["#items", [["", "0"], ["4", "4"], ["4\n4", "8"], ["4\n4\n4", "12"]]], + ["#values", [["", "0"], ["5", "5"], ["5\n5", "10"], ["5\n5\n5", "15"]]], + ["#items", [["5", "5"], ["5\n5", "10"], ["5\n5\n5", "15"], ["", "0"]]] + ] + for selector, output_spec in actions: + dash_duo.find_element(selector).click() + for j, (content, output) in enumerate(output_spec): + dash_duo.wait_for_text_to_equal(".content{}".format(j), content) + dash_duo.wait_for_text_to_equal(".output{}".format(j), output) + + assert not dash_duo.get_logs() + + +def test_cbmo003_multi_all(dash_duo): + app = dash.Dash(__name__) + + app.layout = html.Div(children=[ + html.Button("items", id="items"), + html.Button("values", id="values"), + html.Div(id="content1"), + html.Hr(), + html.Div(id="content2"), + html.Hr(), + html.Div("Output init", id="output"), + ]) + + @app.callback( + [Output("content1", "children"), Output("content2", "children")], + [Input("items", "n_clicks")] + ) + def content(n1): + c1 = [html.Div(id={"i": i}) for i in range(((n1 or 0) + 2) % 4)] + c2 = [html.Div(id={"j": j}) for j in range((n1 or 0) % 3)] + return c1, c2 + + @app.callback( + [Output({"i": ALL}, "children"), Output({"j": ALL}, "children")], + [Input("values", "n_clicks")] + ) + def content_inner(n2): + # this variant does NOT get called with empty ALL + # the second callback handles outputting 0 in that case. + # if it were to be called throw an error so we'll see it in get_logs + n1i = len(dash.callback_context.outputs_list[0]) + n1j = len(dash.callback_context.outputs_list[1]) + if not n1i + n1j: + raise ValueError("should not be called with no outputs!") + return [n2 or 0] * n1i, [(n2 or 0) + 2] * n1j + + @app.callback( + Output("output", "children"), + [Input({"i": ALL}, "children"), Input({"j": ALL}, "children")] + ) + def out2(ci, cj): + return sum(ci) + sum(cj) + + dash_duo.start_server(app) + + dash_duo.wait_for_text_to_equal("#content1", "0\n0") + dash_duo.wait_for_text_to_equal("#content2", "") + dash_duo.wait_for_text_to_equal("#output", "0") + + actions = [ + ["#values", "1\n1", "", "2"], + ["#items", "1\n1\n1", "3", "6"], + ["#values", "2\n2\n2", "4", "10"], + ["#items", "", "4\n4", "8"], + ["#values", "", "5\n5", "10"], + ["#items", "3", "", "3"], + ["#values", "4", "", "4"], + ["#items", "4\n4", "6", "14"], + ["#values", "5\n5", "7", "17"], + ["#items", "5\n5\n5", "7\n7", "29"], + ["#values", "6\n6\n6", "8\n8", "34"], + # all empty! we'll see an error logged if the callback was fired + ["#items", "", "", "0"], + ["#values", "", "", "0"], + ["#items", "7", "9", "16"] + ] + for selector, content1, content2, output in actions: + dash_duo.find_element(selector).click() + dash_duo.wait_for_text_to_equal("#content1", content1) + dash_duo.wait_for_text_to_equal("#content2", content2) + dash_duo.wait_for_text_to_equal("#output", output) + + assert not dash_duo.get_logs()