Skip to content

Commit

Permalink
[Logs] Amend lazy imports in logs_shared plugin (#164102)
Browse files Browse the repository at this point in the history
## Summary

As part of #161151 a [selection of
component imports were made
lazy](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/public/index.ts#L52)
and wrapped with a [`dynamic` wrapper
component](https://github.com/elastic/kibana/blob/main/x-pack/plugins/logs_shared/common/dynamic.tsx#L22).
Unfortunately some of these imports did not adhere to the rules of
React's `lazy` imports (needing a `default` export, no named imports
etc), and the `dynamic` wrapper seems to have suppressed error
information that would have been available via using `lazy` directly.

Only the anomaly and categories log entry examples (in the expanded
rows) were affected by this, as the stream and embeddable import from
locations that were backed by a `default` export (and those top level
components don't import from that particular index file lower in the
hierarchy). For imports that weren't backed by a `default` I've added
them, and where necessary moved components to new files if needed (since
it's one `default` per file).

Also open to suggestions of ways we can alter the `<dynamic />`
component and maintain the error safety 🤔

## Examples

Without these changes:

![Screenshot 2023-08-16 at 17 35
50](https://github.com/elastic/kibana/assets/471693/78aa0300-109e-40b5-b64f-6574a547cbf3)

Warning using `lazy` directly without the `dynamic` wrapper:

![Screenshot 2023-08-16 at 17 36
27](https://github.com/elastic/kibana/assets/471693/a71e3c72-cf3a-4846-9ee9-df70c1729b03)

## Testing

- Check all instances render correctly (stream, embeddable uses, and ML
page log entry examples).
  • Loading branch information
Kerry350 authored Aug 17, 2023
1 parent a5cceb3 commit a96785c
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 55 deletions.
4 changes: 3 additions & 1 deletion x-pack/plugins/logs_shared/common/dynamic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

import React, { lazy, Suspense } from 'react';

type LoadableComponent = () => any;
type LoadableComponent = () => Promise<{
default: React.ComponentType<any>;
}>;

interface DynamicOptions {
fallback?: React.ReactNode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import { i18n } from '@kbn/i18n';
import React from 'react';
import { transparentize } from 'polished';

import { euiStyled } from '@kbn/kibana-react-plugin/common';
import {
Expand All @@ -16,7 +15,6 @@ import {
LogEntryColumnWidth,
LogEntryColumnWidths,
} from './log_entry_column';
import { ASSUMED_SCROLLBAR_WIDTH } from './vertical_scroll_panel';
import { useLogPositionStateContext } from '../../../containers/logs/log_position';
import { localizedDate } from '../../../../common/formatters/datetime';
import {
Expand All @@ -25,6 +23,7 @@ import {
isMessageColumnRenderConfiguration,
isFieldColumnRenderConfiguration,
} from '../../../utils/log_column_render_configuration';
import LogColumnHeadersWrapper from './column_headers_wrapper';

export const LogColumnHeaders: React.FunctionComponent<{
columnConfigurations: LogColumnRenderConfiguration[];
Expand Down Expand Up @@ -112,22 +111,6 @@ export const LogColumnHeader: React.FunctionComponent<{
</LogColumnHeaderWrapper>
);

export const LogColumnHeadersWrapper = euiStyled.div.attrs((props) => ({
role: props.role ?? 'row',
}))`
align-items: stretch;
display: flex;
flex-direction: row;
flex-wrap: nowrap;
justify-content: flex-start;
overflow: hidden;
padding-right: ${ASSUMED_SCROLLBAR_WIDTH}px;
border-bottom: ${(props) => props.theme.eui.euiBorderThin};
box-shadow: 0 2px 2px -1px ${(props) => transparentize(0.3, props.theme.eui.euiColorLightShade)};
position: relative;
z-index: 1;
`;

const LogColumnHeaderWrapper = euiStyled(LogEntryColumn).attrs((props) => ({
role: props.role ?? 'columnheader',
}))`
Expand All @@ -146,3 +129,6 @@ const LogColumnHeaderContent = euiStyled(LogEntryColumnContent)`
text-overflow: clip;
white-space: pre;
`;

// eslint-disable-next-line import/no-default-export
export default LogColumnHeader;
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { euiStyled } from '@kbn/kibana-react-plugin/common';
import { transparentize } from 'polished';
import { ASSUMED_SCROLLBAR_WIDTH } from './vertical_scroll_panel';

export const LogColumnHeadersWrapper = euiStyled.div.attrs((props) => ({
role: props.role ?? 'row',
}))`
align-items: stretch;
display: flex;
flex-direction: row;
flex-wrap: nowrap;
justify-content: flex-start;
overflow: hidden;
padding-right: ${ASSUMED_SCROLLBAR_WIDTH}px;
border-bottom: ${(props) => props.theme.eui.euiBorderThin};
box-shadow: 0 2px 2px -1px ${(props) =>
transparentize(0.3, props.theme.eui.euiColorLightShade)};
position: relative;
z-index: 1;
`;

// eslint-disable-next-line import/no-default-export
export default LogColumnHeadersWrapper;
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
export type { LogEntryStreamItem } from './item';
export type { LogEntryColumnWidths } from './log_entry_column';

export { LogColumnHeader, LogColumnHeadersWrapper } from './column_headers';
export { LogColumnHeader } from './column_headers';
export { LogColumnHeadersWrapper } from './column_headers_wrapper';
export { iconColumnId, LogEntryColumn, useColumnWidths } from './log_entry_column';
export { LogEntryContextMenu } from './log_entry_context_menu';
export { LogEntryFieldColumn } from './log_entry_field_column';
export { LogEntryMessageColumn } from './log_entry_message_column';
export { LogEntryRowWrapper } from './log_entry_row';
export { LogEntryRowWrapper } from './log_entry_row_wrapper';
export { LogEntryTimestampColumn } from './log_entry_timestamp_column';
export { ScrollableLogTextStreamView } from './scrollable_log_text_stream_view';
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,6 @@ export const useColumnWidths = ({
[columnWidths, CharacterDimensionsProbe]
);
};

// eslint-disable-next-line import/no-default-export
export default LogEntryColumn;
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,6 @@ const AbsoluteWrapper = euiStyled.div`
const ButtonWrapper = euiStyled.div`
transform: translate(-6px, -6px);
`;

// eslint-disable-next-line import/no-default-export
export default LogEntryContextMenu;
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,6 @@ const FieldColumnContent = euiStyled(LogEntryColumnContent)<LogEntryColumnConten
? preWrappedContentStyle
: unwrappedContentStyle};
`;

// eslint-disable-next-line import/no-default-export
export default LogEntryFieldColumn;
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,6 @@ const renderMessageSegments = (messageSegments: LogMessagePart[]): string => {
})
.join(' ');
};

// eslint-disable-next-line import/no-default-export
export default LogEntryMessageColumn;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { i18n } from '@kbn/i18n';
import { euiStyled } from '@kbn/kibana-react-plugin/common';
import { ObservabilityTriggerId } from '@kbn/observability-shared-plugin/common';
import {
useUiTracker,
Expand All @@ -29,8 +28,8 @@ import { iconColumnId, LogEntryColumn, LogEntryColumnWidths } from './log_entry_
import { LogEntryContextMenu } from './log_entry_context_menu';
import { LogEntryFieldColumn } from './log_entry_field_column';
import { LogEntryMessageColumn } from './log_entry_message_column';
import { LogEntryRowWrapper } from './log_entry_row_wrapper';
import { LogEntryTimestampColumn } from './log_entry_timestamp_column';
import { highlightedContentStyle, hoveredContentStyle, monospaceTextStyle } from './text_styles';

const MENU_LABEL = i18n.translate('xpack.logsShared.logEntryItemView.logEntryActionsMenuToolTip', {
defaultMessage: 'View actions for line',
Expand Down Expand Up @@ -85,7 +84,6 @@ export const LogEntryRow = memo(

const setItemIsHovered = useCallback(() => setIsHovered(true), []);
const setItemIsNotHovered = useCallback(() => setIsHovered(false), []);

const openFlyout = useCallback(
() => openFlyoutWithItem?.(logEntry.id),
[openFlyoutWithItem, logEntry.id]
Expand Down Expand Up @@ -263,26 +261,5 @@ export const LogEntryRow = memo(
}
);

interface LogEntryRowWrapperProps {
scale: TextScale;
isHighlighted?: boolean;
}

export const LogEntryRowWrapper = euiStyled.div.attrs(() => ({
role: 'row',
}))<LogEntryRowWrapperProps>`
align-items: stretch;
color: ${(props) => props.theme.eui.euiTextColor};
display: flex;
flex-direction: row;
flex-wrap: nowrap;
justify-content: flex-start;
overflow: hidden;
${(props) => monospaceTextStyle(props.scale)};
${(props) => (props.isHighlighted ? highlightedContentStyle : '')}
&:hover {
${hoveredContentStyle}
}
`;
// eslint-disable-next-line import/no-default-export
export default LogEntryRow;
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { euiStyled } from '@kbn/kibana-react-plugin/common';
import { TextScale } from '../../../../common/log_text_scale';
import { highlightedContentStyle, hoveredContentStyle, monospaceTextStyle } from './text_styles';

export const LogEntryRowWrapper = euiStyled.div.attrs(() => ({
role: 'row',
}))<LogEntryRowWrapperProps>`
align-items: stretch;
color: ${(props) => props.theme.eui.euiTextColor};
display: flex;
flex-direction: row;
flex-wrap: nowrap;
justify-content: flex-start;
overflow: hidden;
${(props) => monospaceTextStyle(props.scale)};
${(props) => (props.isHighlighted ? highlightedContentStyle : '')}
&:hover {
${hoveredContentStyle}
}
`;

interface LogEntryRowWrapperProps {
scale: TextScale;
isHighlighted?: boolean;
}

// eslint-disable-next-line import/no-default-export
export default LogEntryRowWrapper;
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ const TimestampColumnContent = euiStyled(LogEntryColumnContent)`
text-overflow: clip;
white-space: pre;
`;

// eslint-disable-next-line import/no-default-export
export default LogEntryTimestampColumn;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import React, { Fragment } from 'react';
import React, { Fragment, GetDerivedStateFromProps } from 'react';
import moment from 'moment';

import { euiStyled } from '@kbn/kibana-react-plugin/common';
Expand Down Expand Up @@ -73,10 +73,10 @@ export class ScrollableLogTextStreamView extends React.PureComponent<
ScrollableLogTextStreamViewProps,
ScrollableLogTextStreamViewState
> {
public static getDerivedStateFromProps(
nextProps: ScrollableLogTextStreamViewProps,
prevState: ScrollableLogTextStreamViewState
): Partial<ScrollableLogTextStreamViewState> | null {
public static getDerivedStateFromProps: GetDerivedStateFromProps<
ScrollableLogTextStreamViewProps,
ScrollableLogTextStreamViewState
> = (nextProps, prevState) => {
const hasNewTarget = nextProps.target && nextProps.target !== prevState.target;
const hasItems = nextProps.items.length > 0;

Expand Down Expand Up @@ -118,7 +118,7 @@ export class ScrollableLogTextStreamView extends React.PureComponent<
return {
isScrollLocked: false,
};
}
};

constructor(props: ScrollableLogTextStreamViewProps) {
super(props);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/logs_shared/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const LogColumnHeader = dynamic(
() => import('./components/logging/log_text_stream/column_headers')
);
export const LogColumnHeadersWrapper = dynamic(
() => import('./components/logging/log_text_stream/column_headers')
() => import('./components/logging/log_text_stream/column_headers_wrapper')
);
export const LogEntryColumn = dynamic(
() => import('./components/logging/log_text_stream/log_entry_column')
Expand All @@ -68,7 +68,7 @@ export const LogEntryMessageColumn = dynamic(
() => import('./components/logging/log_text_stream/log_entry_message_column')
);
export const LogEntryRowWrapper = dynamic(
() => import('./components/logging/log_text_stream/log_entry_row')
() => import('./components/logging/log_text_stream/log_entry_row_wrapper')
);
export const LogEntryTimestampColumn = dynamic(
() => import('./components/logging/log_text_stream/log_entry_timestamp_column')
Expand Down

0 comments on commit a96785c

Please sign in to comment.