Skip to content

Commit 87e8346

Browse files
authored
fix: Fix dataframe output duplicate rendering (#136)
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
1 parent 065b4c0 commit 87e8346

File tree

6 files changed

+114
-51
lines changed

6 files changed

+114
-51
lines changed

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
DeepnoteButtonMetadataSchema,
3131
DeepnoteSqlMetadata
3232
} from './deepnoteSchemas';
33+
import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes';
3334

3435
export type InputBlockType =
3536
| 'input-text'
@@ -189,7 +190,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation
189190
const defaultMetadata: DeepnoteSqlMetadata = {
190191
deepnote_variable_name: deepnoteVariableName,
191192
deepnote_return_variable_type: 'dataframe',
192-
sql_integration_id: 'deepnote-dataframe-sql'
193+
sql_integration_id: DATAFRAME_SQL_INTEGRATION_ID
193194
};
194195

195196
// Determine the index where to insert the new cell (below current selection or at the end)

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
import { IDisposable } from '../../platform/common/types';
2222
import * as notebookUpdater from '../../kernels/execution/notebookUpdater';
2323
import { createMockedNotebookDocument } from '../../test/datascience/editor-integration/helpers';
24+
import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes';
2425

2526
suite('DeepnoteNotebookCommandListener', () => {
2627
let commandListener: DeepnoteNotebookCommandListener;
@@ -734,7 +735,7 @@ suite('DeepnoteNotebookCommandListener', () => {
734735
);
735736
assert.equal(
736737
newCell.metadata.sql_integration_id,
737-
'deepnote-dataframe-sql',
738+
DATAFRAME_SQL_INTEGRATION_ID,
738739
'Should have correct sql integration id'
739740
);
740741

src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
IntegrationType,
1010
PostgresIntegrationConfig,
1111
BigQueryIntegrationConfig,
12+
DATAFRAME_SQL_INTEGRATION_ID,
1213
SnowflakeIntegrationConfig,
1314
SnowflakeAuthMethods
1415
} from './integrationTypes';
@@ -74,7 +75,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => {
7475
const uri = Uri.file('/test/notebook.deepnote');
7576
const notebook = createMockNotebook(uri, [
7677
createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM df', {
77-
sql_integration_id: 'deepnote-dataframe-sql'
78+
sql_integration_id: DATAFRAME_SQL_INTEGRATION_ID
7879
})
7980
]);
8081

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import * as React from 'react';
2+
3+
import { DataframeMetadata, DataframeRenderer } from './DataframeRenderer';
4+
import { RendererContext } from 'vscode-notebook-renderer';
5+
6+
export interface Metadata {
7+
cellId?: string;
8+
cellIndex?: number;
9+
executionCount: number;
10+
metadata?: DataframeMetadata;
11+
outputType: string;
12+
}
13+
14+
export function DataframeRendererContainer({
15+
context,
16+
outputJson,
17+
outputMetadata
18+
}: {
19+
context: RendererContext<unknown>;
20+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
21+
outputJson: any;
22+
outputMetadata: Metadata;
23+
}) {
24+
console.log(`Dataframe renderer - received data with ${Object.keys(outputJson).length} keys`);
25+
26+
console.log('[DataframeRenderer] Full metadata', outputMetadata);
27+
28+
const dataFrameMetadata = outputMetadata?.metadata as DataframeMetadata | undefined;
29+
const cellId = outputMetadata?.cellId;
30+
31+
console.log(`[DataframeRenderer] Extracted cellId: ${cellId}`);
32+
33+
return <DataframeRenderer context={context} data={outputJson} metadata={dataFrameMetadata} cellId={cellId} />;
34+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import React from 'react';
2+
import type { FallbackProps } from 'react-error-boundary';
3+
4+
export function ErrorFallback({ error }: FallbackProps) {
5+
return (
6+
<div
7+
style={{
8+
padding: '16px',
9+
color: 'var(--vscode-errorForeground)',
10+
backgroundColor: 'var(--vscode-inputValidation-errorBackground)',
11+
border: '1px solid var(--vscode-inputValidation-errorBorder)',
12+
borderRadius: '4px',
13+
fontFamily: 'var(--vscode-font-family)',
14+
fontSize: '13px'
15+
}}
16+
>
17+
<div style={{ fontWeight: 'bold', marginBottom: '8px' }}>Error rendering dataframe</div>
18+
<div style={{ marginBottom: '8px' }}>{error.message}</div>
19+
<details style={{ marginTop: '8px', cursor: 'pointer' }}>
20+
<summary>Stack trace</summary>
21+
<pre
22+
style={{
23+
marginTop: '8px',
24+
padding: '8px',
25+
backgroundColor: 'var(--vscode-editor-background)',
26+
overflow: 'auto',
27+
fontSize: '11px',
28+
whiteSpace: 'pre-wrap',
29+
wordBreak: 'break-word'
30+
}}
31+
>
32+
{error.stack}
33+
</pre>
34+
</details>
35+
</div>
36+
);
37+
}

src/webviews/webview-side/dataframe-renderer/index.ts

Lines changed: 37 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -5,61 +5,50 @@ import * as ReactDOM from 'react-dom';
55

66
import type { ActivationFunction, OutputItem, RendererContext } from 'vscode-notebook-renderer';
77

8-
import { DataframeMetadata, DataframeRenderer } from './DataframeRenderer';
9-
10-
interface Metadata {
11-
cellId?: string;
12-
cellIndex?: number;
13-
executionCount: number;
14-
metadata?: DataframeMetadata;
15-
outputType: string;
16-
}
8+
import { ErrorBoundary } from 'react-error-boundary';
9+
import { ErrorFallback } from './ErrorBoundary';
10+
import { DataframeRendererContainer, Metadata } from './DataframeRendererContainer';
1711

1812
export const activate: ActivationFunction = (context: RendererContext<unknown>) => {
1913
return {
2014
renderOutputItem(outputItem: OutputItem, element: HTMLElement) {
21-
console.log(`Dataframe renderer - rendering output item: ${outputItem.id}`);
22-
try {
23-
const data = outputItem.json();
24-
25-
console.log(`Dataframe renderer - received data with ${Object.keys(data).length} keys`);
26-
27-
const metadata = outputItem.metadata as Metadata | undefined;
28-
29-
console.log('[DataframeRenderer] Full metadata', metadata);
30-
31-
const dataFrameMetadata = metadata?.metadata as DataframeMetadata | undefined;
32-
const cellId = metadata?.cellId;
33-
const cellIndex = metadata?.cellIndex;
34-
35-
console.log(`[DataframeRenderer] Extracted cellId: ${cellId}, cellIndex: ${cellIndex}`);
36-
37-
const root = document.createElement('div');
38-
39-
element.appendChild(root);
40-
41-
ReactDOM.render(
42-
React.createElement(DataframeRenderer, {
15+
ReactDOM.render(
16+
React.createElement(
17+
ErrorBoundary,
18+
{
19+
FallbackComponent: ErrorFallback,
20+
onError: (error, info) => {
21+
console.error('Vega renderer error:', error, info);
22+
}
23+
},
24+
React.createElement(DataframeRendererContainer, {
4325
context,
44-
data,
45-
metadata: dataFrameMetadata,
46-
cellId,
47-
cellIndex
48-
}),
49-
root
50-
);
51-
} catch (error) {
52-
console.error(`Error rendering dataframe: ${error}`);
53-
const errorDiv = document.createElement('div');
54-
errorDiv.style.padding = '10px';
55-
errorDiv.style.color = 'var(--vscode-errorForeground)';
56-
errorDiv.textContent = `Error rendering dataframe: ${error}`;
57-
element.appendChild(errorDiv);
58-
}
26+
outputJson: outputItem.json(),
27+
outputMetadata: outputItem.metadata as Metadata
28+
})
29+
),
30+
element
31+
);
5932
},
6033

61-
disposeOutputItem(_id?: string) {
62-
// Cleanup if needed
34+
disposeOutputItem(id?: string) {
35+
// If undefined, all cells are being removed.
36+
if (id == null) {
37+
for (let i = 0; i < document.children.length; i++) {
38+
const child = document.children.item(i);
39+
if (child == null) {
40+
continue;
41+
}
42+
ReactDOM.unmountComponentAtNode(child);
43+
}
44+
return;
45+
}
46+
47+
const element = document.getElementById(id);
48+
if (element == null) {
49+
return;
50+
}
51+
ReactDOM.unmountComponentAtNode(element);
6352
}
6453
};
6554
};

0 commit comments

Comments
 (0)