Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions zeppelin-web-angular/projects/zeppelin-react/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions zeppelin-web-angular/projects/zeppelin-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"@ant-design/icons": "5.4.0",
"@zeppelin/sdk": "file:../zeppelin-sdk",
"ansi-to-react": "6.2.6",
"highlight.js": "^9.15.8",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@types/highlight.js looks like it needs to come along. The shell (zeppelin-web-angular/package.json) carries both, highlight.js in dependencies and "@types/highlight.js": "^9.12.3" in devDependencies, while only the runtime half landed here. npm ci in this package alone then gives:

src/components/renderers/HTMLRenderer.tsx(34,16): error TS7016:
  Could not find a declaration file for module 'highlight.js'

@voidmatcha voidmatcha Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch. I reproduced TS7016 with a clean install of projects/zeppelin-react. The root install was masking it because the Angular shell already provides @types/highlight.js.

Since highlight.js is now a direct dependency of the React package, it should also add @types/highlight.js@^9.12.3 to its devDependencies and regenerate the lockfile. Thanks for catching this!

Other than this, the PR looks good to me. I’ll approve once it’s fixed.

"antd": "5.21.0",
"chart.js": "^4.5.1",
"date-fns": "^3.6.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { render } from '@testing-library/react';
import { describe, expect, it, vi } from 'vitest';
import { HTMLRenderer } from './HTMLRenderer';

// A file of its own: the mock is hoisted over the whole module, so it cannot
// share one with the specs that need highlighting to work.
vi.mock('highlight.js', () => {
throw new Error('chunk load failed');
});

describe('HTMLRenderer when the highlight chunk fails', () => {
it('leaves the code block unhighlighted instead of raising an unhandled rejection', async () => {
const { container } = render(<HTMLRenderer html="<pre><code>const x = 1;</code></pre>" />);

// Give the rejected import a turn to settle; nothing must escape it.
await vi.waitFor(() => expect(container.querySelector('pre code')).not.toBeNull());

expect(container.querySelector('pre code')!.classList.contains('hljs')).toBe(false);
expect(container.textContent).toContain('const x = 1;');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { render, screen, waitFor } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { HTMLRenderer } from './HTMLRenderer';

// jsdom never runs scripts, so the execution this component exists for is out of reach here and belongs in e2e.
// What these specs pin is the fidelity of the swap:
// attributes, body and position survive it. They do not prove the swap happened,
// because a script parsed out of innerHTML already carries all three.
// Only the async assertion below distinguishes a rebuilt script from an inert one.
describe('HTMLRenderer', () => {
it('renders the markup it is given', () => {
render(<HTMLRenderer html="<p>rendered output</p>" />);

expect(screen.getByText('rendered output')).toBeTruthy();
});

it('carries the original attributes onto the replacement script', () => {
const { container } = render(
<HTMLRenderer html='<script type="text/javascript" data-mark="kept" src="lib.js"></script>' />
);

const script = container.querySelector('script')!;
expect(script.getAttribute('type')).toBe('text/javascript');
expect(script.getAttribute('data-mark')).toBe('kept');
expect(script.getAttribute('src')).toBe('lib.js');
});

it('keeps the script body so the replacement has something to run', () => {
const { container } = render(<HTMLRenderer html="<script>window.answer = 42;</script>" />);

expect(container.querySelector('script')!.textContent).toBe('window.answer = 42;');
});

it('forces async off even when the source markup asked for it', () => {
// A library and the code using it must not arrive out of order.
const { container } = render(<HTMLRenderer html='<script async src="lib.js"></script>' />);

expect(container.querySelector('script')!.async).toBe(false);
});

it('leaves each script where it was among the surrounding markup', () => {
const { container } = render(
<HTMLRenderer html='<p>before</p><script id="one"></script><p>between</p><script id="two"></script>' />
);

const ids = Array.from(container.querySelectorAll('.inner-html > *')).map(node => node.id || node.tagName);
expect(ids).toEqual(['P', 'one', 'P', 'two']);
});

it('highlights a code block, matching what the Angular renderer does', async () => {
const { container } = render(<HTMLRenderer html="<pre><code>const x = 1;</code></pre>" />);

// highlight.js arrives through a dynamic import, so the class lands a tick
// later. Which language it guesses is its own business and not pinned here.
await waitFor(() => expect(container.querySelector('pre code')!.classList.contains('hljs')).toBe(true));
});

it('replaces the previous output when the html changes', () => {
const { rerender } = render(<HTMLRenderer html="<p>first</p>" />);

rerender(<HTMLRenderer html="<p>second</p>" />);

expect(screen.queryByText('first')).toBeNull();
expect(screen.getByText('second')).toBeTruthy();
});

it('renders nothing visible for empty html', () => {
const { container } = render(<HTMLRenderer html="" />);

expect(container.textContent).toBe('');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ export const HTMLRenderer = ({ html }: HTMLRendererProps) => {
// Highlight code blocks (matches Angular: result.component.ts renderHTML)
const codeEle = container.querySelector('pre code');
if (codeEle) {
import('highlight.js').then(({ default: hljs }) => {
hljs.highlightBlock(codeEle as HTMLElement);
});
import('highlight.js')
.then(({ default: hljs }) => {
hljs.highlightBlock(codeEle as HTMLElement);
})
// Without this a failed chunk is an unhandled rejection; the cost is an unhighlighted block.
.catch(() => undefined);
}

const scripts = Array.from(container.querySelectorAll('script'));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { render, screen } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { DatasetType, ParagraphConfigResults, ParagraphIResultsMsgItem } from '@zeppelin/sdk';
import { SingleResultRenderer } from './SingleResultRenderer';

const result = (type: DatasetType, data: string): ParagraphIResultsMsgItem => ({ type, data });

const TABLE_DATA = 'name\tage\nalice\t30';

// Index 0 stays a table, index 1 draws a chart, so reading the wrong entry shows.
const configs = {
0: { graph: { mode: 'table' } },
1: { graph: { mode: 'multiBarChart' } }
} as unknown as ParagraphConfigResults;

describe('SingleResultRenderer', () => {
it('renders TEXT as text, leaving markup in it literal', () => {
// Markup in the payload is what separates this arm from HTML: routing TEXT to
// HTMLRenderer would parse the tag away instead of showing it.
render(<SingleResultRenderer index={0} result={result(DatasetType.TEXT, 'line one <b>not bold</b>')} />);

expect(screen.getByText(/line one <b>not bold<\/b>/)).toBeTruthy();
});

it('renders TABLE through the visualization', () => {
render(<SingleResultRenderer index={0} result={result(DatasetType.TABLE, TABLE_DATA)} />);

// Only that the arm was taken. The visualization's display-mode state is a
// known defect (projects/zeppelin-react/AGENTS.md), so nothing here pins it.
expect(screen.getByText('alice')).toBeTruthy();
expect(screen.getByRole('button', { name: /Bar Chart/ })).toBeTruthy();
});

it('hands the visualization the display config for its own result index', () => {
// Both indices are rendered: index 1 alone would pass against a hard-coded [1],
// and the positive assertion keeps an absent chart from reading as success.
const table = render(
<SingleResultRenderer index={0} config={configs} result={result(DatasetType.TABLE, TABLE_DATA)} />
);
expect(screen.getByText('alice')).toBeTruthy();
table.unmount();

render(<SingleResultRenderer index={1} config={configs} result={result(DatasetType.TABLE, TABLE_DATA)} />);
expect(screen.getByRole('button', { name: /Table/ })).toBeTruthy();
expect(screen.queryByText('alice')).toBeNull();
});
Comment on lines +46 to +58

@tbonelee tbonelee Aug 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: what about placing index={0} alongside it as a contrast? Right now the only assertion is that alice is absent, so this still passes if the TABLE arm returns null outright.

Suggested change
it('hands the visualization the display config for its own result index', () => {
render(<SingleResultRenderer index={1} config={configs} result={result(DatasetType.TABLE, TABLE_DATA)} />);
expect(screen.queryByText('alice')).toBeNull();
});
it('hands the visualization the display config for its own result index', () => {
const table = render(
<SingleResultRenderer index={0} config={configs} result={result(DatasetType.TABLE, TABLE_DATA)} />
);
expect(screen.getByText('alice')).toBeTruthy();
table.unmount();
render(<SingleResultRenderer index={1} config={configs} result={result(DatasetType.TABLE, TABLE_DATA)} />);
expect(screen.queryByText('alice')).toBeNull();
});

unmount() because both renders stay in the document until afterEach(cleanup), so screen would otherwise search across them. rerender() does not work either: TableVisualization reads the config only into its useState initial value, so the mounted mode survives.

What do you think?


it('renders IMG as a base64 png', () => {
render(<SingleResultRenderer index={0} result={result(DatasetType.IMG, 'QUJD')} />);

expect(screen.getByRole('img').getAttribute('src')).toBe('data:image/png;base64,QUJD');
});

it('renders HTML as markup rather than as text', () => {
render(<SingleResultRenderer index={0} result={result(DatasetType.HTML, '<p>markup output</p>')} />);

expect(screen.getByText('markup output').tagName).toBe('P');
});

it('tells the user that ANGULAR results are unsupported here', () => {
render(<SingleResultRenderer index={0} result={result(DatasetType.ANGULAR, 'anything')} />);

expect(screen.getByText('Angular Component')).toBeTruthy();
expect(screen.getByText(/not supported in React environment/)).toBeTruthy();
});

it('renders nothing for a type it has no renderer for', () => {
// NETWORK is declared by the SDK and reaches the default arm.
const { container } = render(<SingleResultRenderer index={0} result={result(DatasetType.NETWORK, 'graph')} />);

expect(container.innerHTML).toBe('');
});
});
Loading