Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing defining potentially non existent components in the running keyword #2898

Merged
merged 6 commits into from
Jun 25, 2024

Conversation

aGitForEveryone
Copy link
Contributor

closes #2897

When using the running keyword in (regular) callbacks, the app crashes if a component is used that does not exist in the layout. Added a check to only continue with component update it the component was found in the dashboard. Full explanation in #2897.

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • Added check to only allow component updates in running if component is defined
    • Added test to verify correct behavior
  • I have run the tests locally and they passed. I can't run the full test suite locally. My PC can't handle it.
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md

If a component defined in running does not exist, the componentPath in updateComponent will be undefined. Added a check that skips the update if that is the case. Otherwise the dashboard will crash as it will try to perform operations on the undefined object that it cannot perform.
@@ -334,6 +334,11 @@ function updateComponent(component_id: any, props: any) {
return function (dispatch: any, getState: any) {
const paths = getState().paths;
const componentPath = getPath(paths, component_id);
if (typeof componentPath === 'undefined') {
// Can't find the component that was defined in the running keyword,
// Let's skip the component to prevent the dashboard from crashing.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to throw an error unless config.suppress_callback_exceptions=True. It is in state.config, could refactor line 335 to const {paths,config} = getState();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check for that and threw an error when suppress_callback_exceptions=False

…hrow error

Non-existent running components will now only silently passed if surpress_callback_exceptions=True. Otherwise, an error will be thrown. Note: even if an error is thrown, the callback execution is not stopped. Only the side-update of the running component is ignored.
…g components and not suppressing callback exceptions
Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Looks good

@T4rk1n T4rk1n merged commit bbd013c into plotly:dev Jun 25, 2024
3 checks passed
@aGitForEveryone aGitForEveryone deleted the running-non-existent-component branch August 30, 2024 07:13
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] Callback running keyword doesn't work when component is not in layout
2 participants