Skip to content

Commit

Permalink
refactor: final tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
mdjastrzebski committed Jun 14, 2024
1 parent 78cc069 commit 6c6f88c
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 32 deletions.
2 changes: 1 addition & 1 deletion packages/compare/src/compare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ function compareResults(current: MeasureResults, baseline: MeasureResults | null
.filter((item) => Math.abs(item.countDiff) > COUNT_DIFF_THRESHOLD)
.sort((a, b) => b.countDiff - a.countDiff);
const renderIssues = withCurrent.filter(
(item) => item.current.initialUpdateCount || item.current.redundantUpdates?.length
(item) => item.current.issues?.initialUpdateCount || item.current.issues?.redundantUpdates?.length
);
added.sort((a, b) => a.name.localeCompare(b.name));
removed.sort((a, b) => a.name.localeCompare(b.name));
Expand Down
17 changes: 9 additions & 8 deletions packages/compare/src/output/console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,15 @@ function printRegularLine(entry: CompareEntry) {

function printRenderIssuesLine(entry: CompareEntry | AddedEntry) {
const issues = [];
if (entry.current.initialUpdateCount !== 0) {
issues.push(formatInitialUpdates(entry.current.initialUpdateCount));

const initialUpdateCount = entry.current.issues?.initialUpdateCount;
if (initialUpdateCount) {
issues.push(formatInitialUpdates(initialUpdateCount));
}

if (entry.current.redundantUpdates?.length !== 0) {
issues.push(formatRedundantUpdates(entry.current.redundantUpdates));
const redundantUpdates = entry.current.issues?.redundantUpdates;
if (redundantUpdates?.length) {
issues.push(formatRedundantUpdates(redundantUpdates));
}

logger.log(` - ${entry.name}: ${issues.join(' | ')}`);
Expand All @@ -89,16 +92,14 @@ function printRemovedLine(entry: RemovedEntry) {
);
}

export function formatInitialUpdates(count: number | undefined) {
if (count == null) return '?';
export function formatInitialUpdates(count: number) {
if (count === 0) return '-';
if (count === 1) return '1 initial update 🔴';

return `${count} initial updates 🔴`;
}

export function formatRedundantUpdates(redundantUpdates: number[] | undefined) {
if (redundantUpdates == null) return '?';
export function formatRedundantUpdates(redundantUpdates: number[]) {
if (redundantUpdates.length === 0) return '-';
if (redundantUpdates.length === 1) return `1 redundant update (${redundantUpdates.join(', ')}) 🔴`;

Expand Down
4 changes: 2 additions & 2 deletions packages/compare/src/output/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ function buildRedundantRendersTable(entries: Array<CompareEntry | AddedEntry>) {
const tableHeader = ['Name', 'Initial Updates', 'Redundant Updates'] as const;
const rows = entries.map((entry) => [
entry.name,
formatInitialUpdates(entry.current.initialUpdateCount),
formatRedundantUpdates(entry.current.redundantUpdates),
formatInitialUpdates(entry.current.issues?.initialUpdateCount),
formatRedundantUpdates(entry.current.issues?.redundantUpdates),
]);

return markdownTable([tableHeader, ...rows]);
Expand Down
8 changes: 6 additions & 2 deletions packages/compare/src/type-schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export const MeasureEntryScheme = z.object({
/** Array of measured render/execution counts for each run. */
counts: z.array(z.number()),

initialUpdateCount: z.number().optional(),
redundantUpdates: z.array(z.number()).optional(),
issues: z.optional(
z.object({
initialUpdateCount: z.number().optional(),
redundantUpdates: z.array(z.number()).optional(),
})
),
});
34 changes: 17 additions & 17 deletions packages/measure/src/__tests__/measure-renders.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ test('measureRenders correctly measures regular renders', async () => {
};

const results = await measureRenders(<Regular />, { scenario, writeFile: false });
expect(results.initialUpdateCount).toBe(0);
expect(results.redundantUpdates).toEqual([]);
expect(results.issues.initialUpdateCount).toBe(0);
expect(results.issues.redundantUpdates).toEqual([]);
});

const InitialUpdates = ({ updateCount }: { updateCount: number }) => {
Expand All @@ -114,17 +114,17 @@ const InitialUpdates = ({ updateCount }: { updateCount: number }) => {

test('measureRenders detects redundant initial renders', async () => {
const results = await measureRenders(<InitialUpdates updateCount={1} />, { writeFile: false });
expect(results.initialUpdateCount).toBe(1);
expect(results.redundantUpdates).toEqual([]);
expect(results.issues.initialUpdateCount).toBe(1);
expect(results.issues.redundantUpdates).toEqual([]);
});

test('measureRenders detects multiple redundant initial renders', async () => {
const results = await measureRenders(<InitialUpdates updateCount={5} />, { writeFile: false });
expect(results.initialUpdateCount).toBe(5);
expect(results.redundantUpdates).toEqual([]);
expect(results.issues.initialUpdateCount).toBe(5);
expect(results.issues.redundantUpdates).toEqual([]);
});

const RedundantUpdates = () => {
const RenderIssues = () => {
const [count, setCount] = React.useState(0);
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [_, forceRender] = React.useState(0);
Expand All @@ -148,9 +148,9 @@ test('measureRenders detects redundant updates', async () => {
await fireEvent.press(screen.getByText('Redundant'));
};

const results = await measureRenders(<RedundantUpdates />, { scenario, writeFile: false });
expect(results.redundantUpdates).toEqual([1]);
expect(results.initialUpdateCount).toBe(0);
const results = await measureRenders(<RenderIssues />, { scenario, writeFile: false });
expect(results.issues.redundantUpdates).toEqual([1]);
expect(results.issues.initialUpdateCount).toBe(0);
});

test('measureRenders detects multiple redundant updates', async () => {
Expand All @@ -160,9 +160,9 @@ test('measureRenders detects multiple redundant updates', async () => {
await fireEvent.press(screen.getByText('Redundant'));
};

const results = await measureRenders(<RedundantUpdates />, { scenario, writeFile: false });
expect(results.redundantUpdates).toEqual([1, 3]);
expect(results.initialUpdateCount).toBe(0);
const results = await measureRenders(<RenderIssues />, { scenario, writeFile: false });
expect(results.issues.redundantUpdates).toEqual([1, 3]);
expect(results.issues.initialUpdateCount).toBe(0);
});

const AsyncMacroTaskEffect = () => {
Expand All @@ -181,8 +181,8 @@ const AsyncMacroTaskEffect = () => {

test('ignores async macro-tasks effect', async () => {
const results = await measureRenders(<AsyncMacroTaskEffect />, { writeFile: false });
expect(results.initialUpdateCount).toBe(0);
expect(results.redundantUpdates).toEqual([]);
expect(results.issues.initialUpdateCount).toBe(0);
expect(results.issues.redundantUpdates).toEqual([]);
});

const AsyncMicrotaskEffect = () => {
Expand All @@ -206,8 +206,8 @@ const AsyncMicrotaskEffect = () => {

test('ignores async micro-tasks effect', async () => {
const results = await measureRenders(<AsyncMicrotaskEffect />, { writeFile: false });
expect(results.initialUpdateCount).toBe(0);
expect(results.redundantUpdates).toEqual([]);
expect(results.issues.initialUpdateCount).toBe(0);
expect(results.issues.redundantUpdates).toEqual([]);
});

function Wrapper({ children }: React.PropsWithChildren<{}>) {
Expand Down
6 changes: 4 additions & 2 deletions packages/measure/src/measure-renders.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ async function measureRendersInternal(

return {
...processRunResults(runResults, warmupRuns),
initialUpdateCount: initialRenderCount - 1,
redundantUpdates: detectRedundantUpdates(renderJsonTrees, initialRenderCount),
issues: {
initialUpdateCount: initialRenderCount - 1,
redundantUpdates: detectRedundantUpdates(renderJsonTrees, initialRenderCount),
},
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/measure/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ export interface MeasureResults {
}

export interface MeasureRendersResults extends MeasureResults {
issues: RenderIssues;
}

export interface RenderIssues {
/**
* Update renders (re-renders) that happened immediately after component was created
* e.g., synchronous `useEffects` containing `setState`.
Expand Down

0 comments on commit 6c6f88c

Please sign in to comment.