Skip to content

Commit

Permalink
[Log Explorer] Fix broken data grid on columns update (#165679)
Browse files Browse the repository at this point in the history
## 📓  Summary

Closes #165558 

The issue was caused by a particular condition caused by the overridden
`data` service that the plugin injects in the Discover App.
Reverse engineering from the blank screen when updating the columns:
1. The `DiscoverHistogramLayout` is rendered conditionally only [when a
`searchSessionId`
exists](https://github.com/elastic/kibana/blob/main/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx#L47).
2. The `searchSessionId` is initialized with a default value and exists
until [the `data.search.session.clear()` function is
invoked](https://github.com/elastic/kibana/blob/main/src/plugins/discover/public/application/main/discover_main_app.tsx#L82).
3. Being this effect cleanup callback is invoked when
`data.search.session` changes in its reference, the issue is caused by
the [injected
service](https://github.com/elastic/kibana/blob/main/x-pack/plugins/log_explorer/public/components/log_explorer/log_explorer.tsx#L52-L62).
4. The root cause is that each time a property is read from the proxied
`data` service, new nested Proxies are created, triggering the useEffect
cleanup function since the new Proxy has a new reference.

The implemented solution adds an enhanced version of
`createPropertyGetProxy` that keeps the created proxy as a singleton,
storing the original value in a cache by a passed key.

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
  • Loading branch information
tonyghiani and Marco Antonio Ghiani authored Sep 6, 2023
1 parent 1b0c095 commit 55936a8
Showing 1 changed file with 12 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { ScopedHistory } from '@kbn/core-application-browser';
import { DataPublicPluginStart, ISearchStart, ISessionService } from '@kbn/data-plugin/public';
import { DataPublicPluginStart } from '@kbn/data-plugin/public';
import { DiscoverStart } from '@kbn/discover-plugin/public';
import React from 'react';
import {
Expand Down Expand Up @@ -50,13 +50,17 @@ export const createLogExplorer = ({
* are no-ops.
*/
const createDataServiceProxy = (data: DataPublicPluginStart) => {
const noOpEnableStorage = () => {};

const sessionServiceProxy = createPropertyGetProxy(data.search.session, {
enableStorage: () => noOpEnableStorage,
});

const searchServiceProxy = createPropertyGetProxy(data.search, {
session: () => sessionServiceProxy,
});

return createPropertyGetProxy(data, {
search: (searchService: ISearchStart) =>
createPropertyGetProxy(searchService, {
session: (sessionService: ISessionService) =>
createPropertyGetProxy(sessionService, {
enableStorage: () => () => {},
}),
}),
search: () => searchServiceProxy,
});
};

0 comments on commit 55936a8

Please sign in to comment.