Fix running= with an ALL wildcard crashing when no components match - #3957
Draft
Mohit-Ak wants to merge 2 commits into
Draft
Fix running= with an ALL wildcard crashing when no components match#3957Mohit-Ak wants to merge 2 commits into
Mohit-Ak wants to merge 2 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: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 throwsstate.paths.objs[idKey] is undefined. The callback never gets to run.What was going wrong
getAllPMCIdsindexesstate.paths.objs[idKey]and calls.mapon the result.paths.objsonly has a key for an id shape that is actually rendered somewhere, so on a page with none of those components the lookup returnsundefinedand.mapthrows. The two other places that do this same lookup —resolveDepsindependencies_ts.tsandgetPathinpaths.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.
replacePMCdecides whether a wildcard was expanded by checkingextras.length:An
ALLthat legitimately matches zero components expands to an empty list, which is falsy, so control falls through toreturn [replaced]— andreplacedonly ever received the non-wildcard keys, because the wildcard branch writes toextrasinstead. So once the crash is gone you get[{type: 'loading'}]back: an id with theidkey missing entirely.sideUpdatethen 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 madesideUpdatedrop pattern-matching outputs that resolve to nothing, since in that casecomponentIdstill 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 coversMATCH,ALL, plain concrete ids, and the two empty-page cases.Before the source change, with the tests in place:
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:
Lint is clean on the touched files (
eslint0 errors,prettier --list-differentempty).tsc --noEmitreports the same set of pre-existingnode_modulestype errors before and after — none from these files.Fixes #3297