Skip to content

Commit 44f74d6

Browse files
authored
fix: preact toSorted usage (#380)
1 parent d710727 commit 44f74d6

File tree

8 files changed

+65
-42
lines changed

8 files changed

+65
-42
lines changed

packages/scan/src/auto.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import './polyfills';
12
// Prioritize bippy side-effect
23
import 'bippy';
34

packages/scan/src/core/instrumentation.ts

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -187,31 +187,6 @@ export function fastSerialize(value: unknown, depth = 0): string {
187187
return str;
188188
}
189189

190-
export const getPropsChanges = (fiber: Fiber) => {
191-
const changes: Array<Change> = [];
192-
193-
const prevProps = fiber.alternate?.memoizedProps || {};
194-
const nextProps = fiber.memoizedProps || {};
195-
196-
const allKeys = new Set([
197-
...Object.keys(prevProps),
198-
...Object.keys(nextProps),
199-
]);
200-
for (const propName in allKeys) {
201-
// const prevValue = prevProps?.[propName];
202-
const nextValue = nextProps?.[propName];
203-
204-
const change: Change = {
205-
type: ChangeReason.Props,
206-
name: propName,
207-
value: nextValue,
208-
};
209-
changes.push(change);
210-
}
211-
212-
return changes;
213-
};
214-
215190
export const getStateChanges = (fiber: Fiber): StateChange[] => {
216191
if (!fiber) return [];
217192
const changes: StateChange[] = [];
@@ -457,10 +432,10 @@ export function getRenderData(fiber: Fiber) {
457432
}
458433

459434
export function setRenderData(fiber: Fiber, value: RenderData) {
460-
const type = getType(fiber.type)
435+
const type = getType(fiber.type);
461436
const id = getFiberIdentifier(fiber);
462437
let keyMap = renderDataMap.get(type as object);
463-
438+
464439
if (!keyMap) {
465440
keyMap = new Map();
466441
renderDataMap.set(type as object, keyMap);

packages/scan/src/core/notifications/performance.ts

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
Fiber,
33
getDisplayName,
44
getTimings,
5+
hasMemoCache,
56
isHostFiber,
67
traverseFiber,
78
} from 'bippy';
@@ -69,6 +70,8 @@ export type FiberRenders = Record<
6970
parents: Set<string>;
7071
selfTime: number;
7172
totalTime: number;
73+
hasMemoCache: boolean;
74+
wasFiberRenderMount: boolean;
7275
nodeInfo: Array<{
7376
selfTime: number;
7477
element: Element;
@@ -471,20 +474,11 @@ const getTargetInteractionDetails = (target: Element) => {
471474

472475
const componentPath = getInteractionPath(associatedFiber);
473476

474-
// const childrenTree = collectFiberSubtree(associatedFiber, 20); // this can be expensive if not limited
475-
476-
// const firstChildSvg = Object.entries(childrenTree).find(([name, {isSvg }]) => isSvg)
477-
478-
// const firstSvg =
479-
// associatedFiber.type === "svg"
480-
// ? getFirstNameFromAncestor(associatedFiber)
481-
// : Object.entries(childrenTree).find(([name, {isSvg }]) => isSvg)
482-
483-
// lowkey i have an idea
484477
return {
485478
componentPath,
486479
childrenTree: {},
487480
componentName,
481+
elementFiber: associatedFiber,
488482
};
489483
};
490484

@@ -898,6 +892,8 @@ export const listenForRenders = (
898892
};
899893
fiberRenders[displayName] = {
900894
renderCount: 1,
895+
hasMemoCache: hasMemoCache(fiber),
896+
wasFiberRenderMount: wasFiberRenderMount(fiber),
901897
parents: parents,
902898
selfTime,
903899
totalTime,
@@ -933,6 +929,9 @@ export const listenForRenders = (
933929
changesCounts: new Map<string | number, number>(),
934930
};
935931

932+
existing.wasFiberRenderMount =
933+
existing.wasFiberRenderMount || wasFiberRenderMount(fiber);
934+
existing.hasMemoCache = existing.hasMemoCache || hasMemoCache(fiber);
936935
existing.changes = {
937936
fiberProps: mergeSectionData(
938937
existing.changes?.fiberProps || emptySection,
@@ -993,3 +992,24 @@ const mergeSectionData = (
993992

994993
return mergedSection;
995994
};
995+
996+
const wasFiberRenderMount = (fiber: Fiber) => {
997+
if (!fiber.alternate) {
998+
return true;
999+
}
1000+
1001+
const prevFiber = fiber.alternate;
1002+
1003+
const wasMounted =
1004+
prevFiber &&
1005+
prevFiber.memoizedState != null &&
1006+
prevFiber.memoizedState.element != null &&
1007+
prevFiber.memoizedState.isDehydrated !== true;
1008+
1009+
const isMounted =
1010+
fiber.memoizedState != null &&
1011+
fiber.memoizedState.element != null &&
1012+
fiber.memoizedState.isDehydrated !== true;
1013+
1014+
return !wasMounted && isMounted;
1015+
};

packages/scan/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import './polyfills';
12
// Bippy has a side-effect that installs the hook.
23
import 'bippy';
34

packages/scan/src/polyfills.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
if (!Array.prototype.toSorted) {
2+
Object.defineProperty(Array.prototype, 'toSorted', {
3+
value: function <T>(this: Array<T>, compareFn?: (a: T, b: T) => number): Array<T> {
4+
return [...this].sort(compareFn);
5+
},
6+
writable: true,
7+
configurable: true,
8+
});
9+
}

packages/scan/src/web/views/notifications/data.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { createContext } from 'preact';
22
import { SetStateAction } from 'preact/compat';
33
import { Dispatch, useContext } from 'preact/hooks';
44
import { HIGH_SEVERITY_FPS_DROP_TIME } from '~core/notifications/event-tracking';
5+
import { getFiberFromElement } from '../inspector/utils';
6+
import { hasMemoCache } from 'bippy';
57

68
export type GroupedFiberRender = {
79
id: string;
@@ -18,6 +20,8 @@ export type GroupedFiberRender = {
1820
elements: Array<Element>; // can't do a weak set because need to iterate over them......
1921
deletedAll: boolean;
2022
parents: Set<string>;
23+
hasMemoCache: boolean;
24+
wasFiberRenderMount: boolean;
2125
};
2226
export const getComponentName = (path: Array<string>) => {
2327
const filteredPath = path.filter((item) => item.length > 2);
@@ -72,11 +76,22 @@ export type InteractionTiming = {
7276
frameDraw: number | null;
7377
};
7478

75-
export const isRenderMemoizable = (gropedFiberRender: GroupedFiberRender) => {
79+
export const isRenderMemoizable = (
80+
groupedFiberRender: GroupedFiberRender,
81+
): boolean => {
82+
if (groupedFiberRender.wasFiberRenderMount) {
83+
// no amount of memoization can prevent a mount render
84+
return false;
85+
}
86+
// this shouldn't be needed, it implies we either are tracking renders wrong, are tracking changes wrong, or are not tracking some other "state" that can cause re-renders, but its a better fallback than failing
87+
if (groupedFiberRender.hasMemoCache) {
88+
return false;
89+
}
90+
7691
return (
77-
gropedFiberRender.changes.context.length === 0 &&
78-
gropedFiberRender.changes.props.length === 0 &&
79-
gropedFiberRender.changes.state.length === 0
92+
groupedFiberRender.changes.context.length === 0 &&
93+
groupedFiberRender.changes.props.length === 0 &&
94+
groupedFiberRender.changes.state.length === 0
8095
);
8196
};
8297

packages/scan/src/web/views/notifications/notifications.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ const getGroupedFiberRenders = (fiberRenders: FiberRenders) => {
2626
name: render.nodeInfo[0].name, // invariant, at least one exists,
2727
deletedAll: false,
2828
parents: render.parents,
29+
hasMemoCache: render.hasMemoCache,
30+
wasFiberRenderMount: render.wasFiberRenderMount,
2931
// it would be nice if we calculated the % of components memoizable, but this would have to be calculated downstream before it got aggregated
3032
elements: render.nodeInfo.map((node) => node.element),
3133
changes: {

packages/website/tailwind.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const config: Config = {
2121
700: 'oklch(41.78% 0.117 287.1 / <alpha-value>)',
2222
800: 'oklch(34.52% 0.047 290.52 / <alpha-value>)',
2323
900: 'oklch(24.54% 0.004 308.28 / <alpha-value>)',
24-
950: 'oklch(18.22% 0 NaN / <alpha-value>)',
24+
950: 'oklch(18.22% 0 308.28 / <alpha-value>)',
2525
},
2626
},
2727
},

0 commit comments

Comments
 (0)