Skip to content
Merged
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
2 changes: 2 additions & 0 deletions packages/benchmarks/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
traces/
# Benchmark JSON results and reports (generated by bench:json / bench:report)
results/
# Reassure performance test output (generated by perf / perf:baseline)
.reassure/
7 changes: 6 additions & 1 deletion packages/benchmarks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@
"type": "module",
"scripts": {
"bench": "vitest bench --run",
"bench:browser": "vitest bench --config vitest.config.browser.ts --run"
"bench:browser": "vitest bench --config vitest.config.browser.ts --run",
"perf": "rm -f .reassure/current.perf && vitest --run --config vitest.config.perf.ts",
"perf:baseline": "rm -f .reassure/baseline.perf && REASSURE_OUTPUT_FILE=.reassure/baseline.perf vitest --run --config vitest.config.perf.ts",
Comment on lines +10 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n packages/benchmarks/package.json | head -30

Repository: omniviewdev/ui

Length of output: 1449


🏁 Script executed:

# Check if there are other scripts in the project that handle cross-platform concerns
rg "scripts" -A 20 package.json | head -100

Repository: omniviewdev/ui

Length of output: 1675


🏁 Script executed:

# Look for existing cross-platform patterns (e.g., cross-env, cross-spawn usage)
rg "cross-env|cross-spawn|node scripts" --type json -B 2 -A 2

Repository: omniviewdev/ui

Length of output: 977


🏁 Script executed:

# Check root package.json to understand project's approach
cat -n package.json | head -50

Repository: omniviewdev/ui

Length of output: 2164


🏁 Script executed:

cat packages/benchmarks/scripts/perf-compare.mjs 2>/dev/null | head -50

Repository: omniviewdev/ui

Length of output: 589


🏁 Script executed:

# Check if there's a Node version requirement or Windows-specific handling in the project
git log --oneline -1 && head -5 .gitignore

Repository: omniviewdev/ui

Length of output: 186


Use cross-platform approach for perf scripts.

Lines 10-11 use POSIX-only shell syntax (rm -f and REASSURE_OUTPUT_FILE=... env assignment) that will fail on Windows. Since the project already uses Node scripts for similar tooling (perf-compare.mjs), move the cleanup and environment setup into a Node script to ensure perf and perf:baseline work consistently across platforms.

Cross-platform solution
   "scripts": {
     "bench": "vitest bench --run",
     "bench:browser": "vitest bench --config vitest.config.browser.ts --run",
-    "perf": "rm -f .reassure/current.perf && vitest --run --config vitest.config.perf.ts",
-    "perf:baseline": "rm -f .reassure/baseline.perf && REASSURE_OUTPUT_FILE=.reassure/baseline.perf vitest --run --config vitest.config.perf.ts",
+    "perf": "node scripts/run-perf.mjs current",
+    "perf:baseline": "node scripts/run-perf.mjs baseline",
     "perf:compare": "node scripts/perf-compare.mjs"
   },
// packages/benchmarks/scripts/run-perf.mjs
import { rmSync } from 'node:fs';
import { spawnSync } from 'node:child_process';

const mode = process.argv[2];
const output =
  mode === 'baseline'
    ? '.reassure/baseline.perf'
    : '.reassure/current.perf';

rmSync(output, { force: true });

const env =
  mode === 'baseline'
    ? { ...process.env, REASSURE_OUTPUT_FILE: output }
    : process.env;

const cmd = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm';
const { status } = spawnSync(
  cmd,
  ['exec', 'vitest', '--run', '--config', 'vitest.config.perf.ts'],
  { stdio: 'inherit', env }
);

process.exit(status ?? 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"perf": "rm -f .reassure/current.perf && vitest --run --config vitest.config.perf.ts",
"perf:baseline": "rm -f .reassure/baseline.perf && REASSURE_OUTPUT_FILE=.reassure/baseline.perf vitest --run --config vitest.config.perf.ts",
"perf": "node scripts/run-perf.mjs current",
"perf:baseline": "node scripts/run-perf.mjs baseline",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/benchmarks/package.json` around lines 10 - 11, Replace the
POSIX-only package.json script commands ("perf" and "perf:baseline") with a
cross-platform Node script: create a script file (e.g.,
packages/benchmarks/scripts/run-perf.mjs) that uses rmSync to remove the target
file, sets REASSURE_OUTPUT_FILE in the env when mode is "baseline", and
spawnSync to run vitest (detecting Windows to call "pnpm.cmd" vs "pnpm"); then
update the "perf" script to call node run-perf.mjs and the "perf:baseline"
script to call node run-perf.mjs baseline so the cleanup and env setup are
platform-independent.

"perf:compare": "node scripts/perf-compare.mjs"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
"devDependencies": {
"@callstack/reassure-compare": "^1.4.1",
"@codspeed/vitest-plugin": "^5.2.0",
"@omniview/base-ui": "workspace:*",
"@omniview/editors": "workspace:*",
Expand All @@ -22,6 +26,7 @@
"jsdom": "^25.0.1",
"react": "^19.0.0",
"react-dom": "^19.0.0",
"reassure": "^1.4.1",
"typescript": "^5.9.2",
"vite": "^7.3.1",
"vitest": "^4.0.0"
Expand Down
16 changes: 16 additions & 0 deletions packages/benchmarks/scripts/perf-compare.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Compare Reassure performance results between baseline and current runs.
*
* Usage:
* pnpm perf:baseline # run on base branch, writes .reassure/baseline.perf
* pnpm perf # run on current branch, writes .reassure/current.perf
* pnpm perf:compare # compare and output report
*/
import { compare } from '@callstack/reassure-compare';

await compare({
baselineFile: '.reassure/baseline.perf',
currentFile: '.reassure/current.perf',
outputFile: '.reassure/output.json',
outputFormat: 'all',
});
49 changes: 49 additions & 0 deletions packages/benchmarks/src/__perf__/base-ui/Accordion.perf-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { test } from 'vitest';
import { measureRenders } from 'reassure';
import { fireEvent, screen } from '@testing-library/react';
import { Accordion } from '@omniview/base-ui';
import { ThemeWrapper } from '../utils/theme-wrapper';

function FiveItemAccordion() {
return (
<Accordion animation="none">
{Array.from({ length: 5 }, (_, i) => (
<Accordion.Item key={i} id={`item-${i}`} title={`Section ${i}`}>
Content for section {i}
</Accordion.Item>
))}
</Accordion>
);
}

test('Accordion: mount 5 items', async () => {
await measureRenders(<FiveItemAccordion />, { wrapper: ThemeWrapper });
});

/**
* Expanding one section should NOT re-render sibling sections.
* This catches shared-context over-notification patterns.
*/
test('Accordion: expand one section', async () => {
await measureRenders(<FiveItemAccordion />, {
wrapper: ThemeWrapper,
scenario: async () => {
fireEvent.click(screen.getByText('Section 2'));
},
});
});

/**
* Expanding then collapsing — tests the full toggle cycle.
* Render count should be similar to a single expand.
*/
test('Accordion: expand then collapse', async () => {
await measureRenders(<FiveItemAccordion />, {
wrapper: ThemeWrapper,
scenario: async () => {
const trigger = screen.getByText('Section 2');
fireEvent.click(trigger);
fireEvent.click(trigger);
},
});
});
22 changes: 22 additions & 0 deletions packages/benchmarks/src/__perf__/base-ui/Button.perf-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { test } from 'vitest';
import { measureRenders } from 'reassure';
import { Button } from '@omniview/base-ui';
import { ThemeWrapper } from '../utils/theme-wrapper';

/**
* Button is the simplest interactive component — its render count
* serves as the baseline for all other components.
* Expected: 1 render on mount, 0 issues.
*/
test('Button: mount', async () => {
await measureRenders(<Button>Click me</Button>, { wrapper: ThemeWrapper });
});

test('Button: mount with decorators', async () => {
await measureRenders(
<Button startDecorator={<span>+</span>} endDecorator={<span>→</span>}>
Action
</Button>,
{ wrapper: ThemeWrapper },
);
});
49 changes: 49 additions & 0 deletions packages/benchmarks/src/__perf__/base-ui/Checkbox.perf-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { test } from 'vitest';
import { measureRenders } from 'reassure';
import { fireEvent, screen } from '@testing-library/react';
import { Checkbox } from '@omniview/base-ui';
import { ThemeWrapper } from '../utils/theme-wrapper';

test('Checkbox: mount', async () => {
await measureRenders(
<Checkbox.Item defaultChecked={false}>Accept terms</Checkbox.Item>,
{ wrapper: ThemeWrapper },
);
});

test('Checkbox: toggle checked', async () => {
await measureRenders(
<Checkbox.Item defaultChecked={false}>Accept terms</Checkbox.Item>,
{
wrapper: ThemeWrapper,
scenario: async () => {
const checkbox = screen.getByRole('checkbox');
fireEvent.click(checkbox);
},
},
);
});

/**
* Critical scenario: toggling one checkbox in a list.
* If memoization is missing, ALL checkboxes re-render on a single toggle.
*/
test('Checkbox: toggle one in list of 20', async () => {
const items = Array.from({ length: 20 }, (_, i) => `Option ${i}`);
await measureRenders(
<div>
{items.map((label) => (
<Checkbox.Item key={label} defaultChecked={false}>
{label}
</Checkbox.Item>
))}
</div>,
{
wrapper: ThemeWrapper,
scenario: async () => {
const checkboxes = screen.getAllByRole('checkbox');
fireEvent.click(checkboxes[0]!);
},
},
);
});
59 changes: 59 additions & 0 deletions packages/benchmarks/src/__perf__/base-ui/Dialog.perf-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { useState } from 'react';
import { test } from 'vitest';
import { measureRenders } from 'reassure';
import { fireEvent, screen } from '@testing-library/react';
import { Dialog, Button } from '@omniview/base-ui';
import { ThemeWrapper } from '../utils/theme-wrapper';

function DialogWithTrigger() {
const [open, setOpen] = useState(false);
return (
<>
<Button onClick={() => setOpen(true)}>Open Dialog</Button>
<Dialog open={open} onClose={() => setOpen(false)} size="md">
<Dialog.Title>Confirm Action</Dialog.Title>
<Dialog.Body>Are you sure you want to proceed?</Dialog.Body>
<Dialog.Footer>
<Button onClick={() => setOpen(false)}>Cancel</Button>
<Button onClick={() => setOpen(false)}>Confirm</Button>
</Dialog.Footer>
<Dialog.Close />
</Dialog>
</>
);
}

/**
* Mount cost includes the trigger but NOT the dialog body (open=false).
* Render count should be minimal — just the trigger + null dialog.
*/
test('Dialog: mount (closed)', async () => {
await measureRenders(<DialogWithTrigger />, { wrapper: ThemeWrapper });
});

/**
* Opening the dialog triggers portal mount, backdrop, and content render.
* High render counts here signal excessive setState chains in useEffect.
*/
test('Dialog: open', async () => {
await measureRenders(<DialogWithTrigger />, {
wrapper: ThemeWrapper,
scenario: async () => {
fireEvent.click(screen.getByText('Open Dialog'));
},
});
});

/**
* Full open/close cycle. Render count should be ~2x the open-only count.
* Significantly more signals cleanup-related re-renders.
*/
test('Dialog: open then close', async () => {
await measureRenders(<DialogWithTrigger />, {
wrapper: ThemeWrapper,
scenario: async () => {
fireEvent.click(screen.getByText('Open Dialog'));
fireEvent.click(screen.getByText('Cancel'));
},
});
});
63 changes: 63 additions & 0 deletions packages/benchmarks/src/__perf__/base-ui/Input.perf-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { test } from 'vitest';
import { type ChangeEvent, useState } from 'react';
import { measureRenders } from 'reassure';
import { fireEvent, screen } from '@testing-library/react';
import { Input, TextField } from '@omniview/base-ui';
import { ThemeWrapper } from '../utils/theme-wrapper';

test('Input: mount', async () => {
await measureRenders(
<Input>
<Input.Label>Username</Input.Label>
<Input.Control placeholder="Enter username" />
</Input>,
{ wrapper: ThemeWrapper },
);
});

test('TextField: mount', async () => {
await measureRenders(
<TextField>
<TextField.Label>Email</TextField.Label>
<TextField.Control placeholder="Enter email" />
</TextField>,
{ wrapper: ThemeWrapper },
);
});

/**
* Controlled input wrapper so each fireEvent.change triggers a state update
* and re-render, simulating real per-keystroke behavior.
*/
function ControlledInput() {
const [value, setValue] = useState('');
return (
<Input>
<Input.Label>Username</Input.Label>
<Input.Control
placeholder="Enter username"
data-testid="input"
value={value}
onChange={(e: ChangeEvent<HTMLInputElement>) => setValue(e.target.value)}
/>
</Input>
);
}

/**
* Typing in an input field — each keystroke should cause minimal re-renders.
* High counts here signal uncontrolled→controlled mismatches or expensive
* parent re-renders from onChange.
*/
test('Input: type 10 characters', async () => {
await measureRenders(<ControlledInput />, {
wrapper: ThemeWrapper,
scenario: async () => {
const input = screen.getByTestId('input');
const chars = 'helloworld';
for (let i = 0; i < chars.length; i++) {
fireEvent.change(input, { target: { value: chars.slice(0, i + 1) } });
}
},
});
});
67 changes: 67 additions & 0 deletions packages/benchmarks/src/__perf__/base-ui/Select.perf-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { test } from 'vitest';
import { measureRenders } from 'reassure';
import { fireEvent, screen } from '@testing-library/react';
import { Select } from '@omniview/base-ui';
import { makeOptions } from '../../utils/factories';
import { ThemeWrapper } from '../utils/theme-wrapper';

const options = makeOptions(20);

test('Select: mount with 20 options', async () => {
await measureRenders(
<Select defaultValue={options[0]!.value}>
<Select.Trigger>
<Select.Value placeholder="Choose..." />
</Select.Trigger>
<Select.Portal>
<Select.Positioner>
<Select.Popup>
<Select.List>
{options.map((opt) => (
<Select.Item key={opt.value} value={opt.value}>
{opt.label}
</Select.Item>
))}
</Select.List>
</Select.Popup>
</Select.Positioner>
</Select.Portal>
</Select>,
{ wrapper: ThemeWrapper },
);
});

/**
* Opening a Select triggers portal mount, positioner layout, and popup render.
* High render counts here signal excessive context notifications.
*/
test('Select: open dropdown', async () => {
await measureRenders(
<Select defaultValue={options[0]!.value}>
<Select.Trigger>
<Select.Value placeholder="Choose..." />
</Select.Trigger>
<Select.Portal>
<Select.Positioner>
<Select.Popup>
<Select.List>
{options.map((opt) => (
<Select.Item key={opt.value} value={opt.value}>
{opt.label}
</Select.Item>
))}
</Select.List>
</Select.Popup>
</Select.Positioner>
</Select.Portal>
</Select>,
{
wrapper: ThemeWrapper,
scenario: async () => {
const trigger = screen.getByRole('combobox');
fireEvent.click(trigger);
await screen.findByRole('listbox');
},
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);
});
Loading
Loading