Skip to content

Commit

Permalink
[Discover] Address react warnings for legacy table (elastic#154579)
Browse files Browse the repository at this point in the history 
## Summary

This PR resolves react warnings for the legacy table:
<details><summary>passing undefined to `TotalDocuments` during
loading</summary>
<pre>
ract_devtools_backend.js:2655 Warning: Failed prop type: The prop
`value` is marked as required in `FormattedNumber`, but its value is
`undefined`.
at FormattedNumber
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:149935:5)
at TotalDocuments
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.7.js:70:3)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiFlexItem
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109977:23)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109752:23
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiFlexItem
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109977:23)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109752:23
at DocTableEmbeddable
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.7.js:309:109)
at PseudoLocaleWrapper
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:14199:5)
at IntlProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:149454:5)
at I18nProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:14130:3)
at DiscoverDocTableEmbeddable
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.7.js:235:26)
at SavedSearchEmbeddableComponent
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.7.js:976:3)
at Provider
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/kibanaReact/1.0.0/kibanaReact.plugin.js:1794:15)
at CurrentEuiBreakpointProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:161492:23)
at EuiThemeProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:166432:22)
at EuiCacheProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:138902:20)
at EuiProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:139002:25)
at KibanaThemeProvider
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/kibanaReact/1.0.0/kibanaReact.plugin.js:4625:3)
at PseudoLocaleWrapper
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:14199:5)
at IntlProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:149454:5)
at I18nProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:14130:3)
o
</pre>
</details>

<details><summary>non unique keys for table headers and columns in SQL
mode</summary>
<pre>
ract_devtools_backend.js:2655 Warning: Encountered two children with the
same key, `order_date`. Keys should be unique so that components
maintain their identity across updates. Non-unique keys may cause
children to be duplicated and/or omitted — the behavior is unsupported
and could change in a future version.
    at tr
at TableHeader
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.3.js:3956:3)
    at thead
    at table
at DocTableInfiniteContent
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:15565:3)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.3.js:4710:3
at DocTableInfinite
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:15613:79)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiFlexItem
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109977:23)
at DiscoverDocumentsComponent
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:6758:3)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109752:23
at DiscoverMainContent
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:7485:3)
at InPortal
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/unifiedHistogram/1.0.0/unifiedHistogram.chunk.0.js:980:28)
at UnifiedHistogramLayout
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/unifiedHistogram/1.0.0/unifiedHistogram.chunk.0.js:3694:3)
at
http://localhost:5601/rzv/9007199254740991/bundles/plugin/unifiedHistogram/1.0.0/unifiedHistogram.chunk.0.js:3019:95
    at Suspense
at EuiErrorBoundary
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:108416:81)
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at DiscoverHistogramLayout
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:6956:3)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiPanel
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:136743:23)
at EuiPageContent_Deprecated
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:133907:31)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiFlexItem
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109977:23)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:109752:23
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiPageBody
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:133756:23)
    at div
at
http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:170360:73
at EuiPage
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:133610:23)
at DiscoverLayout
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:7173:3)
at DiscoverMainProvider
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:14340:3)
at DiscoverMainApp
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:12539:5)
at DiscoverMainRoute
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:12703:86)
at Route
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:361914:29)
at Route
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/discover/1.0.0/discover.chunk.6.js:3677:3)
at Switch
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:362120:29)
at Router
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:361543:30)
at EuiErrorBoundary
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:108416:81)
at Provider
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/kibanaReact/1.0.0/kibanaReact.plugin.js:1794:15)
at CurrentEuiBreakpointProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:161492:23)
at EuiThemeProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:166432:22)
at EuiCacheProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:138902:20)
at EuiProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-npm/kbn-ui-shared-deps-npm.dll.js:139002:25)
at KibanaThemeProvider
(http://localhost:5601/rzv/9007199254740991/bundles/plugin/kibanaReact/1.0.0/kibanaReact.plugin.js:4625:3)
at PseudoLocaleWrapper
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:14199:5)
at IntlProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:149454:5)
at I18nProvider
(http://localhost:5601/rzv/9007199254740991/bundles/kbn-ui-shared-deps-src/kbn-ui-shared-deps-src.js:14130:3)
o
</pre>
</details>

The one regarding array keys is interesting: time field column is
rendered twice in the legacy table (as the first column and later on
again) in SQL mode. We could consider removing this duplication in
columns but this would become a breaking change to the existing
behaviour, no? So I went with updating the key for now.

For testing:
- switch to legacy table via Advanced Settings
  - add a saved search to Dashboard
  - check SQL mode on Discover
  • Loading branch information
@jughosta
jughosta authored Apr 20, 2023
1 parent 544f908 commit d079fbb
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/plugins/discover/public/components/doc_table/components/table_header/table_header.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ export function TableHeader({
return (
<tr data-test-subj="docTableHeader" className="kbnDocTableHeader">
<th style={{ width: '24px' }} />
{displayedColumns.map((col) => {
{displayedColumns.map((col, index) => {
return (
<TableHeaderColumn
key={col.name}
key={`${col.name}-${index}`}
{...col}
customLabel={dataView.getFieldByName(col.name)?.customLabel}
isTimeColumn={dataView.timeFieldName === col.name}
Expand Down
7 changes: 4 additions & 3 deletions src/plugins/discover/public/components/doc_table/components/table_row.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ export const TableRow = ({
/>
);
} else {
columns.forEach(function (column: string) {
columns.forEach(function (column: string, index) {
const cellKey = `${column}-${index}`;
if (useNewFieldsApi && !mapping(column) && row.raw.fields && !row.raw.fields[column]) {
const innerColumns = Object.fromEntries(
Object.entries(row.raw.fields).filter(([key]) => {
Expand All @@ -163,7 +164,7 @@ export const TableRow = ({

rowCells.push(
<TableCell
key={column}
key={cellKey}
timefield={false}
sourcefield={true}
formatted={formatTopLevelObject(row, innerColumns, dataView, maxEntries)}
Expand All @@ -182,7 +183,7 @@ export const TableRow = ({
);
rowCells.push(
<TableCell
key={column}
key={cellKey}
timefield={false}
sourcefield={column === '_source'}
formatted={displayField(column)}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/public/components/doc_table/doc_table_embeddable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export const DocTableEmbeddable = (props: DocTableEmbeddableProps) => {
</EuiText>
</EuiFlexItem>
)}
{props.totalHitCount !== 0 && (
{Boolean(props.totalHitCount) && (
<EuiFlexItem grow={false} data-test-subj="toolBarTotalDocsText">
<TotalDocuments totalHitCount={props.totalHitCount} />
</EuiFlexItem>
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/public/embeddable/saved_search_grid.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function DiscoverGridEmbeddable(props: DiscoverGridEmbeddableProps) {
responsive={false}
data-test-subj="embeddedSavedSearchDocTable"
>
{Boolean(props.totalHitCount) && props.totalHitCount !== 0 && (
{Boolean(props.totalHitCount) && (
<EuiFlexItem grow={false} style={{ alignSelf: 'flex-end' }}>
<TotalDocuments totalHitCount={props.totalHitCount} />
</EuiFlexItem>
Expand Down

0 comments on commit d079fbb

Please sign in to comment.