Skip to content

Fix running= with an ALL wildcard crashing when no components match - #3957

Draft
Mohit-Ak wants to merge 2 commits into
plotly:devfrom
Mohit-Ak:fix/running-all-empty-page
Draft

Fix running= with an ALL wildcard crashing when no components match#3957
Mohit-Ak wants to merge 2 commits into
plotly:devfrom
Mohit-Ak:fix/running-all-empty-page

Conversation

@Mohit-Ak

Copy link
Copy Markdown

Callbacks that use a pattern-matching id in running= crash the renderer as soon as you're on a page where none of those components exist. From #3297:

@callback(
    Output('data', 'data'),
    Input('reload-data', 'n_clicks'),
    running=[(Output({'type': 'loading', 'id': ALL}, 'display'), 'show', 'hide')],
)

Click the button while you're on the home page and it works. Navigate to /blank, which renders none of the {'type': 'loading', ...} components, click again, and the browser console throws state.paths.objs[idKey] is undefined. The callback never gets to run.

What was going wrong

getAllPMCIds indexes state.paths.objs[idKey] and calls .map on the result. paths.objs only has a key for an id shape that is actually rendered somewhere, so on a page with none of those components the lookup returns undefined and .map throws. The two other places that do this same lookup — resolveDeps in dependencies_ts.ts and getPath in paths.js — both guard it and return an empty result, so this was the odd one out. Guarding it the same way is the direct fix.

That turned out not to be the whole story though. replacePMC decides whether a wildcard was expanded by checking extras.length:

if (extras.length) {
    return extras;
}
return [replaced];

An ALL that legitimately matches zero components expands to an empty list, which is falsy, so control falls through to return [replaced] — and replaced only ever received the non-wildcard keys, because the wildcard branch writes to extras instead. So once the crash is gone you get [{type: 'loading'}] back: an id with the id key missing entirely. sideUpdate then treats that as a real component id and tries to update it. So I tracked expansion with an explicit flag rather than inferring it from the length, and made sideUpdate drop pattern-matching outputs that resolve to nothing, since in that case componentId still holds the unresolved pattern and isn't safe to use.

The "matches nothing" case is the correct outcome here, not an error — there is simply nothing to show or hide, and the callback should proceed normally. That's also why this doesn't route through the existing "ID running component not found in layout" error: that one is for a concrete id that isn't in the layout, whereas a wildcard matching zero components is expected behaviour when you're on a different page.

Testing

Added dash/dash-renderer/tests/patternMatching.test.js, which had no unit coverage before. It covers MATCH, ALL, plain concrete ids, and the two empty-page cases.

Before the source change, with the tests in place:

getAllPMCIds returns an empty list when no component uses that id shape FAILED
	TypeError: Cannot read properties of undefined (reading 'map')
	    at getAllPMCIds (build/commons.js:3121:34)
replacePMC yields no ids when ALL matches nothing on the current page FAILED
	TypeError: Cannot read properties of undefined (reading 'map')
replacePMC yields no ids when ALLSMALLER matches nothing on the current page FAILED
	TypeError: Cannot read properties of undefined (reading 'map')
TOTAL: 3 FAILED, 40 SUCCESS

That's the same error as the reported one, and the MATCH/ALL/concrete-id cases passed throughout, which is what confines the bug to the empty-match path.

After:

$ npm test
TOTAL: 43 SUCCESS

Lint is clean on the touched files (eslint 0 errors, prettier --list-different empty). tsc --noEmit reports the same set of pre-existing node_modules type errors before and after — none from these files.

Fixes #3297

A callback whose running= argument targets a pattern-matching id would
throw "state.paths.objs[idKey] is undefined" whenever no component with
that id shape was currently rendered - for example after navigating to a
page in a multi-page app that does not contain those components.

getAllPMCIds indexed paths.objs unconditionally, so an id shape that was
never registered produced undefined and blew up on .map. It now returns an
empty list, matching what resolveDeps and getPath already do for the same
lookup.

That alone was not enough: replacePMC used extras.length to decide whether
a wildcard had been expanded, so an expansion that legitimately matched
nothing fell through to returning [replaced] - an id containing only the
non-wildcard keys. sideUpdate would then try to update a component with
that malformed pattern id. replacePMC now tracks expansion explicitly, and
sideUpdate skips pattern-matching outputs that resolve to no components.
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] Using pattern matching ALL for running argument fails when the list is empty

1 participant