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

[Canvas] Chore: remove exception that impairs user experience #32114

Merged
merged 1 commit into from
Mar 3, 2019

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Feb 27, 2019

During the alpha stage, this exception was useful to hunt down mw/thunk based issues, but such exceptions tapered off, and on the off-chance the condition ever happens, it's better to keep the page functional, while the old value gets garbage collected (layout engine state is a plain JSON-serializable object, none of its parts require a destroy method). Also, the ongoing refactoring removes this Map altogether.

@monfera monfera added review v7.0.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v6.7.0 v7.2.0 v6.7.1 v7.0.1 labels Feb 27, 2019
@monfera monfera self-assigned this Feb 27, 2019
@monfera monfera requested a review from w33ble February 27, 2019 10:52
@monfera monfera requested a review from a team as a code owner February 27, 2019 10:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@w33ble w33ble left a comment

Choose a reason for hiding this comment

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

Seems safe enough. As you point out, not checking for that case doesn't really have any negative side effects, and that error hasn't come up in quite some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v6.7.0 v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants