diff --git a/CHANGELOG.md b/CHANGELOG.md index ec744d19e4..79098e7a33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). - [#4027](https://github.com/plotly/dash/pull/4027) Support `async def` hook routes (`dash.hooks.route`) on all backends, stop hook routes from leaking the app context to the caller, and let hook routes and MCP read the request on the `fastapi` backend. - [#4053](https://github.com/plotly/dash/pull/4053) Fix callbacks being registered twice under a reloader (e.g. `uvicorn --reload`) when `Dash()` is given an explicit `name` or is built by an app factory called from the main module. Fixes [#3912](https://github.com/plotly/dash/issues/3912). - [#3980](https://github.com/plotly/dash/pull/3980) Fix the three `before_request` hooks (`Dash._setup_server` and the pages `router_sync` / `router_async`) publishing their "already done" guard flag before the setup work behind it had run. Under a multi-threaded WSGI worker such as `gunicorn -k gthread` (or under an ASGI worker for the async router), a second request arriving mid-setup could observe the flag already set, skip setup, then read `registered_paths` / `callback_map` / the pages router callback while they were still being registered - causing the first burst of component bundle requests after a restart to 500 with `Error loading dependency. "" is not a registered library`, or the pages router to hit `DuplicateCallback` when two workers raced past the guard. Each hook body now runs under a lock (`threading.Lock` for the two sync hooks, an `asyncio.Lock` bound to the running loop for the async router) and only publishes the flag after all work completes. Fixes [#3971](https://github.com/plotly/dash/issues/3971). +- [#3957](https://github.com/plotly/dash/pull/3957) Fix `running=` with a pattern-matching `ALL`/`ALLSMALLER` id crashing the renderer when none of the matching components are on the current page. Fixes [#3297](https://github.com/plotly/dash/issues/3297). - [#3944](https://github.com/plotly/dash/pull/3944) Fix `dash.testing` runner backend detection for wrapped FastAPI/Quart servers so threaded Flask-only options are not passed to ASGI runners. - [#3955](https://github.com/plotly/dash/pull/3955) Unpin `selenium` in the testing requirements (was capped at `<=4.2.0` from 2022) and require `>=4.11.0`, so it can drive current stable Chrome via Selenium Manager and stop the widespread CI flakiness. - [#3881](https://github.com/plotly/dash/pull/3881) Speed up the renderer layout crawl (`crawlLayout` and its callers) by replacing curried-ramda `path`/`pathOr` lookups with direct property access on the hot path: ~16% faster `Patch().append()` into a large container, with no behavior change. diff --git a/dash/dash-renderer/src/actions/callbacks.ts b/dash/dash-renderer/src/actions/callbacks.ts index 83d3fe5e93..8eca117d19 100644 --- a/dash/dash-renderer/src/actions/callbacks.ts +++ b/dash/dash-renderer/src/actions/callbacks.ts @@ -444,16 +444,25 @@ function sideUpdate(outputs: SideUpdateOutput, cb: ICallbackPayload) { let componentId = id, propName, replacedIds = []; + let isPatternMatching = false; if (id.startsWith('{')) { [componentId, propName] = parsePMCId(id); replacedIds = replacePMC(componentId, cb, i, getState); + isPatternMatching = true; } else if (id.includes('.')) { [componentId, propName] = id.split('.'); } const props = propName ? {[propName]: value} : value; + if (isPatternMatching && replacedIds.length === 0) { + // A wildcard that matches nothing currently rendered. + // There is no component to update, and `componentId` still + // holds the unresolved pattern, so it must not be used. + return acc; + } + if (replacedIds.length === 0) { acc.push([componentId, props]); } else if (replacedIds.length === 1) { diff --git a/dash/dash-renderer/src/actions/patternMatching.ts b/dash/dash-renderer/src/actions/patternMatching.ts index 8a3bb01150..10e412e3f2 100644 --- a/dash/dash-renderer/src/actions/patternMatching.ts +++ b/dash/dash-renderer/src/actions/patternMatching.ts @@ -32,7 +32,13 @@ export function parsePMCId(id: string): [any, string | undefined] { export function getAllPMCIds(id: any, state: any, triggerKey: string) { const keysOfIds = keys(id); const idKey = keysOfIds.join(','); - return state.paths.objs[idKey] + // No component with this id shape is currently rendered, so nothing + // matches the wildcard. + const registered = state.paths.objs[idKey]; + if (!registered) { + return []; + } + return registered .map((obj: any) => keysOfIds.reduce((acc, key, i) => { acc[key] = obj.values[i]; @@ -62,9 +68,14 @@ export function replacePMC( getState: any ): any[] { let extras: any = []; + // Whether an ALL/ALLSMALLER key was expanded. `extras` alone cannot tell + // us this, since a wildcard that matches no rendered component expands to + // an empty list -- in which case there is genuinely nothing to update, and + // `replaced` only holds the non-wildcard keys, so it is not a usable id. + let expanded = false; const replaced: any = {}; toPairs(id).forEach(([key, value]) => { - if (extras.length) { + if (expanded) { // All done. return; } @@ -75,16 +86,18 @@ export function replacePMC( replaced[key] = triggerValue; } else if (value.includes('ALL')) { extras = getAllPMCIds(id, getState(), key); + expanded = true; } else if (value.includes('ALLSMALLER')) { extras = getAllPMCIds(id, getState(), key).filter( (obj: any) => obj[key] < triggerValue ); + expanded = true; } } else { replaced[key] = value; } }); - if (extras.length) { + if (expanded) { return extras; } return [replaced]; diff --git a/dash/dash-renderer/tests/patternMatching.test.js b/dash/dash-renderer/tests/patternMatching.test.js new file mode 100644 index 0000000000..b273c1c7a9 --- /dev/null +++ b/dash/dash-renderer/tests/patternMatching.test.js @@ -0,0 +1,121 @@ +import {expect} from 'chai'; +import {beforeEach, describe, it} from 'mocha'; +import {getAllPMCIds, replacePMC} from '../src/actions/patternMatching'; + +// Minimal stand-in for the pieces of the redux state that the pattern +// matching helpers read. +function makeState(objs) { + return {paths: {strs: {}, objs: objs || {}}}; +} + +// A wildcard entry as produced by crawling the layout: `values` is the list +// of id values ordered by the (sorted) id keys. +function entries(keyStr, valueLists) { + return { + [keyStr]: valueLists.map((values, i) => ({ + values, + path: ['props', 'children', i] + })) + }; +} + +const cb = {parsedChangedPropsIds: [{id: 'home1', type: 'loading'}]}; + +describe('getAllPMCIds', () => { + it('returns the matching ids when the wildcard key is registered', () => { + const state = makeState( + entries('id,type', [ + ['home1', 'loading'], + ['home2', 'loading'] + ]) + ); + const result = getAllPMCIds( + {id: ['ALL'], type: 'loading'}, + state, + 'id' + ); + expect(result).to.deep.equal([ + {id: 'home1', type: 'loading'}, + {id: 'home2', type: 'loading'} + ]); + }); + + it('returns an empty list when no component uses that id shape', () => { + // This is the state on a page that renders none of the wildcard + // components: `paths.objs['id,type']` was never populated. + const result = getAllPMCIds( + {id: ['ALL'], type: 'loading'}, + makeState({}), + 'id' + ); + expect(result).to.deep.equal([]); + }); +}); + +describe('replacePMC', () => { + let getState; + + beforeEach(() => { + getState = () => + makeState( + entries('id,type', [ + ['home1', 'loading'], + ['home2', 'loading'] + ]) + ); + }); + + it('expands ALL to every matching component', () => { + const result = replacePMC( + {id: ['ALL'], type: 'loading'}, + cb, + 0, + getState + ); + expect(result).to.deep.equal([ + {id: 'home1', type: 'loading'}, + {id: 'home2', type: 'loading'} + ]); + }); + + it('resolves MATCH against the triggering id', () => { + const result = replacePMC( + {id: ['MATCH'], type: 'loading'}, + cb, + 0, + getState + ); + expect(result).to.deep.equal([{id: 'home1', type: 'loading'}]); + }); + + it('leaves a fully concrete id untouched', () => { + const result = replacePMC( + {id: 'home1', type: 'loading'}, + cb, + 0, + getState + ); + expect(result).to.deep.equal([{id: 'home1', type: 'loading'}]); + }); + + it('yields no ids when ALL matches nothing on the current page', () => { + // Regression test for #3297: navigating to a page that has none of + // the wildcard components used to throw + // `state.paths.objs[idKey] is undefined`, and later returned a + // partial id missing the wildcard key. + const empty = () => makeState({}); + const result = replacePMC({id: ['ALL'], type: 'loading'}, cb, 0, empty); + expect(result).to.deep.equal([]); + }); + + it('yields no ids when ALLSMALLER matches nothing on the current page', () => { + const empty = () => makeState({}); + const result = replacePMC( + {id: ['ALLSMALLER'], type: 'loading'}, + cb, + 0, + empty + ); + expect(result).to.deep.equal([]); + }); +}); diff --git a/tests/integration/callbacks/test_wildcards.py b/tests/integration/callbacks/test_wildcards.py index 704a4f418f..754f935e98 100644 --- a/tests/integration/callbacks/test_wildcards.py +++ b/tests/integration/callbacks/test_wildcards.py @@ -631,6 +631,44 @@ def on_click(_) -> str: ) +def test_cbwc015_running_all_absent_page(dash_duo): + app = Dash(__name__) + app.layout = html.Div( + [ + dcc.Location(id="url"), + dcc.Link("Other", href="/other", id="link"), + html.Button("Go", id="btn"), + html.Div(id="out"), + html.Div(id="page"), + ] + ) + + @app.callback(Output("page", "children"), Input("url", "pathname")) + def render_page(pathname): + if pathname == "/other": + return html.Div("other page", id="other") + return [html.Div(id={"type": "loading", "index": i}) for i in range(2)] + + @app.callback( + Output("out", "children"), + Input("btn", "n_clicks"), + running=[(Output({"type": "loading", "index": ALL}, "children"), "on", "off")], + prevent_initial_call=True, + ) + def on_click(n_clicks): + return f"clicked {n_clicks}" + + dash_duo.start_server(app) + dash_duo.wait_for_element('[id*="loading"]') + + dash_duo.find_element("#link").click() + dash_duo.wait_for_text_to_equal("#other", "other page") + + dash_duo.find_element("#btn").click() + dash_duo.wait_for_text_to_equal("#out", "clicked 1") + assert dash_duo.get_logs() == [] + + def test_cbwc009_match_input_fixed_output(dash_duo): # Issue #2462: allow MATCH in Input with a fixed-id Output. app = Dash(__name__)