diff --git a/CHANGELOG.md b/CHANGELOG.md index 51e3212852..088ca9ca57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). - [#3646](https://github.com/plotly/dash/pull/3646) Remove React 16 support (`16.14.0` is no longer an accepted value for `REACT_VERSION` / `_set_react_version`). ### Fixed +- [#3957](https://github.com/plotly/dash/pull/3957) Fix a `running` argument using a pattern-matching `ALL`/`ALLSMALLER` id crashing the renderer with `state.paths.objs[idKey] is undefined` when none of the matching components are on the current page (for example after navigating to another page in a multi-page app). The wildcard now resolves to an empty set of components and the callback proceeds without any side updates. 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`. The old cap predated Selenium Manager, so the pinned selenium could not drive the current stable Chrome (151+) that CI installs, producing widespread `StaleElementReferenceException`/`TimeoutException` flakiness across the browser-based integration tests. Modern selenium auto-provisions a matching chromedriver, restoring stable CI runs. - Speed up the layout crawl the renderer runs on every path recompute and callback gather (`crawlLayout` and its callers) by replacing curried-ramda `path`/`pathOr` lookups with direct property access on the per-node hot path. Cut a `Patch().append()` into a large container by ~16% and initial render / wildcard resolution by a few percent, with no behavior change. diff --git a/dash/dash-renderer/src/actions/callbacks.ts b/dash/dash-renderer/src/actions/callbacks.ts index 8d065de74c..8a0f3632cd 100644 --- a/dash/dash-renderer/src/actions/callbacks.ts +++ b/dash/dash-renderer/src/actions/callbacks.ts @@ -443,16 +443,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([]); + }); +});