Skip to content

fix: set a consistent key for output elements rather than using the array index MONGOSH-2021 #2382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lerouxb merged 3 commits into from
Feb 24, 2025
Merged

fix: set a consistent key for output elements rather than using the array index MONGOSH-2021 #2382

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
185eddc
set a consistent key for output elements rather than using the array …
lerouxb Feb 21, 2025
2632deb
make key required
lerouxb Feb 21, 2025
0eae9d8
fix tests
lerouxb Feb 24, 2025
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
1 change: 1 addition & 0 deletions packages/browser-repl/src/components/shell-output-line.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type ShellOutputEntryValue = any;
type Glyph = 'ChevronRight' | 'XWithCircle' | 'ChevronLeft';

export interface ShellOutputEntry {
key: number | string;
format: 'input' | 'output' | 'error';
type?: string | null;
value: ShellOutputEntryValue;
Expand Down
4 changes: 2 additions & 2 deletions packages/browser-repl/src/components/shell-output.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ export class ShellOutput extends Component<ShellOutputProps> {
output: PropTypes.arrayOf(PropTypes.any).isRequired,
};

renderLine = (entry: ShellOutputEntry, index: number): JSX.Element => {
renderLine = (entry: ShellOutputEntry): JSX.Element => {
return (
<ShellOutputLine key={`shell-output-entry-${index}`} entry={entry} />
<ShellOutputLine key={`shell-output-entry-${entry.key}`} entry={entry} />
);
};

Expand Down
6 changes: 5 additions & 1 deletion packages/browser-repl/src/components/shell.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ function filterEvaluateCalls(calls: any) {
});
}

let lastKey = 0;

describe('shell', function () {
let fakeRuntime;
let scrollIntoView;
Expand Down Expand Up @@ -79,7 +81,7 @@ describe('shell', function () {

it('takes output', function () {
const output: ShellOutputEntry[] = [
{ format: 'output', value: 'Welcome message goes here' },
{ key: lastKey++, format: 'output', value: 'Welcome message goes here' },
];

render(<ShellWrapper runtime={fakeRuntime} output={output} />);
Expand Down Expand Up @@ -312,6 +314,7 @@ describe('shell', function () {
let output: ShellOutputEntry[] = [];
for (let i = 0; i < 1000; i++) {
output.push({
key: lastKey++,
format: 'output',
type: undefined,
value: 'some result',
Expand Down Expand Up @@ -425,6 +428,7 @@ describe('shell', function () {
it('clears the output when onClearCommand is called', async function () {
let output: ShellOutputEntry[] = [
{
key: lastKey++,
format: 'output',
type: undefined,
value: 'some result',
Expand Down
7 changes: 7 additions & 0 deletions packages/browser-repl/src/components/shell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ const capLengthStart = (elements: unknown[], maxLength: number) => {
elements.splice(maxLength);
};

let lastKey = 0;

const _Shell: ForwardRefRenderFunction<EditorRef | null, ShellProps> = (
{
runtime,
Expand Down Expand Up @@ -262,6 +264,7 @@ const _Shell: ForwardRefRenderFunction<EditorRef | null, ShellProps> = (
...(outputRef.current ?? []),
...result.map(
(entry): ShellOutputEntry => ({
key: lastKey++,
format: 'output',
type: entry.type,
value: entry.printable,
Expand Down Expand Up @@ -354,12 +357,14 @@ const _Shell: ForwardRefRenderFunction<EditorRef | null, ShellProps> = (
runtime.setEvaluationListener(listener);
const result = await runtime.evaluate(code);
outputLine = {
key: lastKey++,
format: 'output',
type: result.type,
value: result.printable,
};
} catch (error) {
outputLine = {
key: lastKey++,
format: 'error',
value: error,
};
Expand All @@ -380,6 +385,7 @@ const _Shell: ForwardRefRenderFunction<EditorRef | null, ShellProps> = (
// don't evaluate empty input, but do add it to the output
if (!code || code.trim() === '') {
newOutputBeforeEval.push({
key: lastKey++,
format: 'input',
value: ' ',
});
Expand All @@ -391,6 +397,7 @@ const _Shell: ForwardRefRenderFunction<EditorRef | null, ShellProps> = (

// add input to output
newOutputBeforeEval.push({
key: lastKey++,
format: 'input',
value: code,
});
Expand Down
16 changes: 13 additions & 3 deletions packages/browser-repl/src/sandbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,26 @@ class DemoServiceProvider {

const runtime = new IframeRuntime(new DemoServiceProvider() as any);

const lotsOfLines: ShellOutputEntry[] = [];
for (let i = 0; i < 99; i++) {
lotsOfLines.push({ key: `entry-${i}`, format: 'output', value: { i } });
}

const IframeRuntimeExample: React.FunctionComponent = () => {
const [darkMode, setDarkMode] = useState(true);
const [redactInfo, setRedactInfo] = useState(false);
const [maxOutputLength, setMaxOutputLength] = useState(1000);
const [maxHistoryLength, setMaxHistoryLength] = useState(1000);
const [maxOutputLength, setMaxOutputLength] = useState(100);
const [maxHistoryLength, setMaxHistoryLength] = useState(100);
const [initialEvaluate, setInitialEvaluate] = useState<string[]>([]);

const [initialText, setInitialText] = useState('');
const [output, setOutput] = useState<ShellOutputEntry[]>([
{ format: 'output', value: { foo: 1, bar: true, buz: function () {} } },
...lotsOfLines,
{
key: 'test',
format: 'output',
value: { foo: 1, bar: true, buz: function () {} },
},
]);
const [history, setHistory] = useState([
'show dbs',
Expand Down