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

[Security Solution] [Investigations] [Tech Debt] removes deepEqual checks in column headers and data providers #130740

Conversation

andrew-goldstein
Copy link
Contributor

[Security Solution] [Investigations] [Tech Debt] removes deepEqual checks in column headers and data providers

This tech debt PR is another entry in a series to remove React.memo deepEqual checks, per the details in #124151

  • It removes deepEqual checks in Timeline's column headers and data providers
  • Files made redundant by the timelines plugin adopting EuiDataGrid are deleted

Methodology

The following techniques were used to ensure that removing the deepEqual checks did NOT result in unexpected re-renders:

  • To understand why components re-rendered, Timeline was profiled with the Record why each component rendered wile profiling setting in the React dev tools Profiler enabled, shown in the (illustrative) screenshot below:

record_why_each_component_rendered

  • Components were temporarily instrumented with counters that incremented every time the component was rendered. Log statements prefixed with [pre] were observed before making changes, per the screenshot below:

pre_change

  • After removing the deepEqual checks, the log prefix was updated to [POST], and the log entries were observed again, per the screenshot below:

post_change

The [pre] and [POST] counters were compared to verify removing the deepEqual checks did NOT introduce unexpected re-renders.

@andrew-goldstein andrew-goldstein added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team v8.3.0 labels Apr 20, 2022
@andrew-goldstein andrew-goldstein requested a review from a team as a code owner April 20, 2022 20:15
@andrew-goldstein andrew-goldstein self-assigned this Apr 20, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Copy link
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

love it lgtm 👍

Copy link
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

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

Files made redundant by the timelines plugin adopting EuiDataGrid are deleted

Nice find 👍

@michaelolo24
Copy link
Contributor

@elasticmachine merge upstream

…l` checks in column headers and data providers

This tech debt PR is another entry in a series to remove `React.memo` `deepEqual` checks, per the details in <elastic#124151>

- It removes `deepEqual` checks in Timeline's column headers and data providers
- Files made redundant by the `timelines` plugin adopting `EuiDataGrid` are deleted

### Methodology

The following techniques were used to ensure that removing the `deepEqual` checks did NOT result in unexpected re-renders:

- To understand why components re-rendered, Timeline was profiled with the `Record why each component rendered wile profiling` setting in the React dev tools Profiler enabled, shown in the (illustrative) screenshot below:

![record_why_each_component_rendered](https://user-images.githubusercontent.com/4459398/158903740-8122e2d3-11a6-4927-916a-f895717835ae.png)

- Components were temporarily instrumented with counters that incremented every time the component was rendered. Log statements prefixed with `[pre]` were observed before making changes, per the screenshot below:

![pre_change](https://user-images.githubusercontent.com/4459398/164310611-3837bd09-0b31-434e-8ef7-94434d35be48.png)

- After removing the `deepEqual` checks, the log prefix was updated to `[POST]`, per the screenshot below:

![post_change](https://user-images.githubusercontent.com/4459398/164310656-f5c82443-2ff4-4e62-8c7b-8fa9dbce5dfd.png)

The `[pre]` and `[POST]` counters were compared to verify removing the `deepEqual` checks did NOT introduce unexpected re-renders.
@andrew-goldstein andrew-goldstein force-pushed the remove-column-header-and-providers-deep-equal branch from f71f7ed to dff43b2 Compare April 21, 2022 17:11
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 4.8MB 4.8MB -852.0B

History

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

cc @andrew-goldstein

@andrew-goldstein andrew-goldstein merged commit 995e63a into elastic:main Apr 21, 2022
@andrew-goldstein andrew-goldstein deleted the remove-column-header-and-providers-deep-equal branch April 21, 2022 18:33
andrew-goldstein added a commit to andrew-goldstein/kibana that referenced this pull request Apr 25, 2022
… code from timelines plugin

This follow-up PR removes redundant code from the `timelines` plugin, identified while implementing elastic#130740
andrew-goldstein added a commit that referenced this pull request Apr 25, 2022
…de from timelines plugin (#130928)

## [Security Solution] [Investigations] [Tech Debt] removes redundant code from the timelines plugin

This follow-up PR removes redundant code from the `timelines` plugin, identified while implementing #130740
dmlemeshko pushed a commit to dmlemeshko/kibana that referenced this pull request May 5, 2022
…ecks in column headers and data providers (elastic#130740)

## [Security Solution] [Investigations] [Tech Debt] removes `deepEqual` checks in column headers and data providers

This tech debt PR is another entry in a series to remove `React.memo` `deepEqual` checks, per the details in <elastic#124151>

- It removes `deepEqual` checks in Timeline's column headers and data providers
- Files made redundant by the `timelines` plugin adopting `EuiDataGrid` are deleted

### Methodology

The following techniques were used to ensure that removing the `deepEqual` checks did NOT result in unexpected re-renders:

- To understand why components re-rendered, Timeline was profiled with the `Record why each component rendered wile profiling` setting in the React dev tools Profiler enabled, shown in the (illustrative) screenshot below:

![record_why_each_component_rendered](https://user-images.githubusercontent.com/4459398/158903740-8122e2d3-11a6-4927-916a-f895717835ae.png)

- Components were temporarily instrumented with counters that incremented every time the component was rendered. Log statements prefixed with `[pre]` were observed before making changes, per the screenshot below:

![pre_change](https://user-images.githubusercontent.com/4459398/164310611-3837bd09-0b31-434e-8ef7-94434d35be48.png)

- After removing the `deepEqual` checks, the log prefix was updated to `[POST]`, and the log entries were observed again, per the screenshot below:

![post_change](https://user-images.githubusercontent.com/4459398/164310656-f5c82443-2ff4-4e62-8c7b-8fa9dbce5dfd.png)

The `[pre]` and `[POST]` counters were compared to verify removing the `deepEqual` checks did NOT introduce unexpected re-renders.
dmlemeshko pushed a commit to dmlemeshko/kibana that referenced this pull request May 5, 2022
…de from timelines plugin (elastic#130928)

## [Security Solution] [Investigations] [Tech Debt] removes redundant code from the timelines plugin

This follow-up PR removes redundant code from the `timelines` plugin, identified while implementing elastic#130740
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
…ecks in column headers and data providers (elastic#130740)

## [Security Solution] [Investigations] [Tech Debt] removes `deepEqual` checks in column headers and data providers

This tech debt PR is another entry in a series to remove `React.memo` `deepEqual` checks, per the details in <elastic#124151>

- It removes `deepEqual` checks in Timeline's column headers and data providers
- Files made redundant by the `timelines` plugin adopting `EuiDataGrid` are deleted

### Methodology

The following techniques were used to ensure that removing the `deepEqual` checks did NOT result in unexpected re-renders:

- To understand why components re-rendered, Timeline was profiled with the `Record why each component rendered wile profiling` setting in the React dev tools Profiler enabled, shown in the (illustrative) screenshot below:

![record_why_each_component_rendered](https://user-images.githubusercontent.com/4459398/158903740-8122e2d3-11a6-4927-916a-f895717835ae.png)

- Components were temporarily instrumented with counters that incremented every time the component was rendered. Log statements prefixed with `[pre]` were observed before making changes, per the screenshot below:

![pre_change](https://user-images.githubusercontent.com/4459398/164310611-3837bd09-0b31-434e-8ef7-94434d35be48.png)

- After removing the `deepEqual` checks, the log prefix was updated to `[POST]`, and the log entries were observed again, per the screenshot below:

![post_change](https://user-images.githubusercontent.com/4459398/164310656-f5c82443-2ff4-4e62-8c7b-8fa9dbce5dfd.png)

The `[pre]` and `[POST]` counters were compared to verify removing the `deepEqual` checks did NOT introduce unexpected re-renders.
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
…de from timelines plugin (elastic#130928)

## [Security Solution] [Investigations] [Tech Debt] removes redundant code from the timelines plugin

This follow-up PR removes redundant code from the `timelines` plugin, identified while implementing elastic#130740
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants