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

[Lens] Fix crash when two layers xychart switches to pie #75267

Merged

Conversation

mbondyra
Copy link
Contributor

Summary

Fixes #74841

The bug was introduced by adding frame dependency to useMemo in suggestionPanel. For now I just reverted the change here and also added a functional test to catch this behaviour in the future.

@mbondyra mbondyra requested a review from a team August 18, 2020 09:03
@mbondyra mbondyra added Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v8.0.0 labels Aug 18, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@@ -204,8 +206,8 @@ export function SuggestionPanel({
: undefined;

return { suggestions: newSuggestions, currentStateExpression: newStateExpression };
// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the main change.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load bundle size

id value diff baseline
lens 858.4KB +254.0B 858.1KB

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested and not getting any crashes, LGTM. Thanks for adding a test as well, this was definitely necessary.

I think we can revisit this later, maybe by memoizing the relevant parts of the frame API itself. If you merge this, could you make sure #74090 is up to date?

@mbondyra mbondyra merged commit e04e05a into elastic:master Aug 18, 2020
mbondyra added a commit to mbondyra/kibana that referenced this pull request Aug 18, 2020
* [Lens] fix crash on two layers and add a functional test with two layers switching to pie chart
mbondyra added a commit that referenced this pull request Aug 18, 2020
…5282)

* [Lens] fix crash on two layers and add a functional test with two layers switching to pie chart
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 18, 2020
* master:
  Skip failing test in CI (elastic#75266)
  [Task Manager] time out work when it overruns in poller (elastic#74980)
  [Drilldowns] misc improvements & fixes (elastic#75276)
  Small README note on bumping memory for builds (elastic#75247)
  [Security Solution][Detections] Adds exception modal tests (elastic#74596)
  [Dashboard] Sample data link does not work (elastic#75262)
  [Dashboard First] Unlink from Library Action With ReferenceOrValueEmbeddable (elastic#74905)
  [Form lib] Fix issue where serializer on fields are called on every change (elastic#75166)
  convert processor labels to sentence case (elastic#75278)
  [Monaco] Refactor the way XJSON grammar checker gets registered (elastic#75160)
  Clarify no documents error message when filtering by is_training (elastic#75227)
  [Lens] Fix crash when two layers xychart  switches to pie (elastic#75267)
  [Observability Homepage] Fix console error because of side effect (elastic#75258)
  [Usage Collection] Add `legacy=true` option to the /api/stats request in the docs (elastic#75146)
  [ML] Functional tests - re-activate DFA test suites (elastic#75257)
  GS providers improvements (elastic#75174)
  [Visualize] First version of by-value visualize editor (elastic#72256)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 19, 2020
…emove-header

* saved-objects/version-on-create: (59 commits)
  remove version when loading sample data
  omit version from SO import/export
  Skip failing test in CI (elastic#75266)
  [Task Manager] time out work when it overruns in poller (elastic#74980)
  [Drilldowns] misc improvements & fixes (elastic#75276)
  Small README note on bumping memory for builds (elastic#75247)
  [Security Solution][Detections] Adds exception modal tests (elastic#74596)
  Revert "Revert "added missing core docs""
  Revert "Revert "added version to saved object bulk creation""
  [Dashboard] Sample data link does not work (elastic#75262)
  [Dashboard First] Unlink from Library Action With ReferenceOrValueEmbeddable (elastic#74905)
  [Form lib] Fix issue where serializer on fields are called on every change (elastic#75166)
  convert processor labels to sentence case (elastic#75278)
  [Monaco] Refactor the way XJSON grammar checker gets registered (elastic#75160)
  Clarify no documents error message when filtering by is_training (elastic#75227)
  [Lens] Fix crash when two layers xychart  switches to pie (elastic#75267)
  [Observability Homepage] Fix console error because of side effect (elastic#75258)
  [Usage Collection] Add `legacy=true` option to the /api/stats request in the docs (elastic#75146)
  [ML] Functional tests - re-activate DFA test suites (elastic#75257)
  GS providers improvements (elastic#75174)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Crashes with two layers and clicking on a suggestion
4 participants