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] Use rules of hooks with linting #65593

Merged
merged 1 commit into from
May 7, 2020

Conversation

wylieconlon
Copy link
Contributor

@wylieconlon wylieconlon commented May 6, 2020

Lens has been breaking one of the rules of hooks, which is that all hooks must execute in each render cycle. This change enables the linting rule and moves some hook registration code and conditional logic to account for this.

Related to #49564

@wylieconlon wylieconlon added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.9.0 labels May 6, 2020
@wylieconlon wylieconlon requested review from timroes, flash1293, mbondyra and a team May 6, 2020 22:51
@elasticmachine
Copy link
Contributor

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

@@ -127,7 +127,7 @@ export function BucketNestingEditor({
defaultMessage: 'Entire data set',
}),
},
...aggColumns,
...aggColumns.map(({ value, text }) => ({ value, text })),
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 actually fixes a console error that @cchaos found, it's not directly related

Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

tested on ff, lgtm 🆗

@wylieconlon wylieconlon merged commit 459daa7 into elastic:master May 7, 2020
@wylieconlon wylieconlon deleted the lens/use-hook-rules branch May 7, 2020 14:01
wylieconlon pushed a commit to wylieconlon/kibana that referenced this pull request May 7, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 7, 2020
* master:
  [ML] Transforms: Fix API error message display for edit flyout. (elastic#65494)
  [SIEM][Detection Engine] Fixes import bug with non existent signals index (elastic#65595)
  [Lens] Use rules of hooks with linting (elastic#65593)
  [ML] Migrate server side Mocha tests to Jest. (elastic#65651)
  Fixes the client to setup SSL with the CA certificates for testing (elastic#65598)
  reduce uptime plugin initial bundle size (elastic#65257)
  [ML] Consolidating shared types and util functions (elastic#65247)
  [Drilldowns] Preserve state when selecting different action factory (elastic#65074)
  Migrate test plugins ⇒ NP (kbn_tp_embeddable_explorer) (elastic#64756)
  Move remaining home assets to the new platform (elastic#65053)
  Change the copy and the id from blacklist to block list for consistency (elastic#65419)
  [ML] Hide selector helper in Anomaly Explorer swimlane (elastic#65522)
  [ML] Fix the limit control on the Anomaly explorer page (elastic#65459)
  [Mappings editor] Add component integration tests (elastic#63853)
  [Logs + Metrics UI] Prevent component errors from breaking the whole UI (elastic#65456)
  [Logs UI] Disable search bar when live stream is on. (elastic#65491)
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 7, 2020
…ponents

* alerting/lazy-load-actions:
  align and style loading indicator
  [ML] Transforms: Fix API error message display for edit flyout. (elastic#65494)
  [SIEM][Detection Engine] Fixes import bug with non existent signals index (elastic#65595)
  [Lens] Use rules of hooks with linting (elastic#65593)
  [ML] Migrate server side Mocha tests to Jest. (elastic#65651)
  Fixes the client to setup SSL with the CA certificates for testing (elastic#65598)
  reduce uptime plugin initial bundle size (elastic#65257)
  [ML] Consolidating shared types and util functions (elastic#65247)
  [Drilldowns] Preserve state when selecting different action factory (elastic#65074)
  Migrate test plugins ⇒ NP (kbn_tp_embeddable_explorer) (elastic#64756)
  Move remaining home assets to the new platform (elastic#65053)
  Change the copy and the id from blacklist to block list for consistency (elastic#65419)
  [ML] Hide selector helper in Anomaly Explorer swimlane (elastic#65522)
  [ML] Fix the limit control on the Anomaly explorer page (elastic#65459)
  [Mappings editor] Add component integration tests (elastic#63853)
  [Logs + Metrics UI] Prevent component errors from breaking the whole UI (elastic#65456)
  [Logs UI] Disable search bar when live stream is on. (elastic#65491)
wylieconlon pushed a commit that referenced this pull request May 7, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
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.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants