[8.6] [Lens][Visualize][Embeddable] Optimize Lens embeddables on error (#144015) (#145695)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Lens][Visualize][Embeddable] Optimize Lens embeddables on error
(#144015)](https://github.com/elastic/kibana/pull/144015)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Liberati","email":"dej611@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-18T08:55:45Z","message":"[Lens][Visualize][Embeddable]
Optimize Lens embeddables on error (#144015)\n\n## Summary\r\n\r\nFixes
#143552\r\n\r\nIn this PR there are some embeddable rendering
optimizations for the\r\nerror case.\r\nLens handles error states
internally without passing to default\r\n`EmbeddablePanel` flows - this
has been made explicit in the Lens\r\n`Embeddable` component. The
embeddable was triggering multiple time the\r\n`render` method in both
successful and failure case: while the\r\nexpression component renderer
had some debouncing/memoize optimization\r\nto reduce the effective
rerenders, the error route had none leading to a\r\ntotal of 3
rerenderings (loading + some changes detected).\r\n\r\nIn this PR I've
managed to reduce the rerenders to 2 (loading + final\r\nstate) with
some gatekeeping on the internal state reporting.\r\nAs for the
`data-rendering-complete` the logic has changed to set it to\r\n`true`
also in Error state, but I would collect some feedback
from\r\n@elastic/kibana-app-services on this side to better understand
the\r\nimpact. Before this change the `data-error` attribute could be
checked\r\nto detect the final state of the panel, but that introduces
some\r\nproblems for Lens panels for the error reporting, hence the need
to have\r\nthe `complete` flag changed.\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[ ] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Uladzislau Lasitsa <vlad.lasitsa@gmail.com>\r\nCo-authored-by: Kibana
Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"87c7b3940b2774c36b94243e325a8f49a2094329","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","Team:AppServicesSv","release_note:skip","Feature:Lens","backport:prev-minor","v8.6.0","v8.7.0"],"number":144015,"url":"https://github.com/elastic/kibana/pull/144015","mergeCommit":{"message":"[Lens][Visualize][Embeddable]
Optimize Lens embeddables on error (#144015)\n\n## Summary\r\n\r\nFixes
#143552\r\n\r\nIn this PR there are some embeddable rendering
optimizations for the\r\nerror case.\r\nLens handles error states
internally without passing to default\r\n`EmbeddablePanel` flows - this
has been made explicit in the Lens\r\n`Embeddable` component. The
embeddable was triggering multiple time the\r\n`render` method in both
successful and failure case: while the\r\nexpression component renderer
had some debouncing/memoize optimization\r\nto reduce the effective
rerenders, the error route had none leading to a\r\ntotal of 3
rerenderings (loading + some changes detected).\r\n\r\nIn this PR I've
managed to reduce the rerenders to 2 (loading + final\r\nstate) with
some gatekeeping on the internal state reporting.\r\nAs for the
`data-rendering-complete` the logic has changed to set it to\r\n`true`
also in Error state, but I would collect some feedback
from\r\n@elastic/kibana-app-services on this side to better understand
the\r\nimpact. Before this change the `data-error` attribute could be
checked\r\nto detect the final state of the panel, but that introduces
some\r\nproblems for Lens panels for the error reporting, hence the need
to have\r\nthe `complete` flag changed.\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[ ] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Uladzislau Lasitsa <vlad.lasitsa@gmail.com>\r\nCo-authored-by: Kibana
Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"87c7b3940b2774c36b94243e325a8f49a2094329"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144015","number":144015,"mergeCommit":{"message":"[Lens][Visualize][Embeddable]
Optimize Lens embeddables on error (#144015)\n\n## Summary\r\n\r\nFixes
#143552\r\n\r\nIn this PR there are some embeddable rendering
optimizations for the\r\nerror case.\r\nLens handles error states
internally without passing to default\r\n`EmbeddablePanel` flows - this
has been made explicit in the Lens\r\n`Embeddable` component. The
embeddable was triggering multiple time the\r\n`render` method in both
successful and failure case: while the\r\nexpression component renderer
had some debouncing/memoize optimization\r\nto reduce the effective
rerenders, the error route had none leading to a\r\ntotal of 3
rerenderings (loading + some changes detected).\r\n\r\nIn this PR I've
managed to reduce the rerenders to 2 (loading + final\r\nstate) with
some gatekeeping on the internal state reporting.\r\nAs for the
`data-rendering-complete` the logic has changed to set it to\r\n`true`
also in Error state, but I would collect some feedback
from\r\n@elastic/kibana-app-services on this side to better understand
the\r\nimpact. Before this change the `data-error` attribute could be
checked\r\nto detect the final state of the panel, but that introduces
some\r\nproblems for Lens panels for the error reporting, hence the need
to have\r\nthe `complete` flag changed.\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[ ] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Uladzislau Lasitsa <vlad.lasitsa@gmail.com>\r\nCo-authored-by: Kibana
Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"87c7b3940b2774c36b94243e325a8f49a2094329"}}]}]
BACKPORT-->

Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts

@@ -38,8 +38,10 @@ export class RenderCompleteDispatcher {
   }
 
   public setEl(el?: HTMLElement) {
-    this.el = el;
-    this.count = 0;
+    if (this.el !== el) {
+      this.el = el;
+      this.count = 0;
+    }
     if (el) this.dispatchInProgress();
   }
 
@@ -61,7 +63,8 @@ export class RenderCompleteDispatcher {
   public dispatchError() {
     if (!this.el) return;
     this.count++;
-    this.el.setAttribute('data-render-complete', 'false');
+    this.el.setAttribute('data-render-complete', 'true');
+    this.el.setAttribute('data-loading', 'false');
     this.el.setAttribute('data-rendering-count', String(this.count));
   }
 

x-pack/plugins/lens/public/embeddable/embeddable.tsx

@@ -522,7 +522,9 @@ export class Embeddable
   }
 
   onContainerStateChanged(containerState: LensEmbeddableInput) {
-    if (this.handleContainerStateChanged(containerState) || this.errors?.length) this.reload();
+    if (this.handleContainerStateChanged(containerState)) {
+      this.reload();
+    }
   }
 
   handleContainerStateChanged(containerState: LensEmbeddableInput): boolean {
@@ -705,12 +707,19 @@ export class Embeddable
 
     this.domNode.setAttribute('data-shared-item', '');
 
+    const error = this.getError();
+
     this.updateOutput({
       ...this.getOutput(),
       loading: true,
-      error: this.getError(),
+      error,
     });
-    this.renderComplete.dispatchInProgress();
+
+    if (error) {
+      this.renderComplete.dispatchError();
+    } else {
+      this.renderComplete.dispatchInProgress();
+    }
 
     const input = this.getInput();