Skip to content

Commit ba34fe9

Browse files
hi-ogawahi-ogawa
authored
fix(tiny-react): fix memo (#168)
1 parent ba8e341 commit ba34fe9

File tree

4 files changed

+90
-90
lines changed

4 files changed

+90
-90
lines changed

‎.changeset/polite-months-cough.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hiogawa/tiny-react": patch
3+
---
4+
5+
fix: fix `memo`

‎packages/tiny-react/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@hiogawa/tiny-react",
3-
"version": "0.0.1",
3+
"version": "0.0.2-pre.1",
44
"homepage": "https://github.com/hi-ogawa/js-utils/tree/main/packages/tiny-react",
55
"repository": {
66
"type": "git",

‎packages/tiny-react/src/compat/index.ts‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1+
import { once } from "@hiogawa/utils";
12
import { useEffect, useRef, useState } from "../hooks";
23
import { render } from "../reconciler";
3-
import { type BNode, EMPTY_NODE, type FC, type VNode } from "../virtual-dom";
4+
import {
5+
type BNode,
6+
EMPTY_NODE,
7+
type FC,
8+
NODE_TYPE_CUSTOM,
9+
type VCustom,
10+
type VNode,
11+
} from "../virtual-dom";
412

513
// non comprehensive compatibility features
614

@@ -39,23 +47,43 @@ export function createRoot(container: Element) {
3947
}
4048

4149
// https://react.dev/reference/react/memo
42-
export function memo<P extends object>(
43-
fc: FC<P>,
44-
propsAreEqual: (
45-
prevProps: Readonly<P>,
46-
nextProps: Readonly<P>
47-
) => boolean = objectShallowEqual
50+
export function memo<P extends {}>(
51+
Fc: FC<P>,
52+
isEqualProps: (prev: {}, next: {}) => boolean = objectShallowEqual
4853
): FC<P> {
4954
function Memo(props: P) {
50-
const prev = useRef<{ props: Readonly<P>; result: VNode } | undefined>(
51-
undefined
52-
);
53-
if (!prev.current || !propsAreEqual(prev.current.props, props)) {
54-
prev.current = { props, result: fc(props) };
55+
// we need to make `VCustom.render` stable,
56+
// but `once(Fc)` has to be invalidated on props change.
57+
// after "identical vnode" optimization is implemented,
58+
// it can be simplified to
59+
// {
60+
// type: NODE_TYPE_CUSTOM,
61+
// render: Fc,
62+
// props,
63+
// }
64+
const [state] = useState(() => {
65+
const state: {
66+
render: FC;
67+
current?: { vnode: VCustom; onceFc: FC };
68+
} = {
69+
render: (props: any) => state.current!.onceFc(props),
70+
};
71+
return state;
72+
});
73+
74+
if (!state.current || !isEqualProps(state.current.vnode.props, props)) {
75+
state.current = {
76+
vnode: {
77+
type: NODE_TYPE_CUSTOM,
78+
render: state.render,
79+
props,
80+
},
81+
onceFc: once(Fc),
82+
};
5583
}
56-
return prev.current.result;
84+
return state.current.vnode;
5785
}
58-
Object.defineProperty(Memo, "name", { value: `Memo(${fc.name})` });
86+
Object.defineProperty(Memo, "name", { value: `Memo(${Fc.name})` });
5987
return Memo;
6088
}
6189

‎packages/tiny-react/src/index.test.ts‎

Lines changed: 42 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
} from "./hooks";
1313
import { render, updateCustomNode } from "./reconciler";
1414
import { sleepFrame } from "./test-utils";
15-
import { getBNodeSlot } from "./virtual-dom";
15+
import { EMPTY_NODE, getBNodeSlot } from "./virtual-dom";
1616

1717
describe(render, () => {
1818
it("basic", () => {
@@ -1613,14 +1613,23 @@ describe("ref", () => {
16131613
});
16141614

16151615
describe(memo, () => {
1616-
it("basic", () => {
1616+
it("basic", async () => {
16171617
const mockFn = vi.fn();
1618+
const mockFnSnapshot = () => mockFn.mock.calls.map((args) => args[0]);
16181619

16191620
const Custom = memo(function Custom(props: {
16201621
label: string;
16211622
value: number;
16221623
}) {
1623-
mockFn(props.label, props.value);
1624+
mockFn(`[render] ${props.label} ${props.value}`);
1625+
1626+
useEffect(() => {
1627+
mockFn(`[effect] ${props.label} ${props.value}`);
1628+
return () => {
1629+
mockFn(`[dispose] ${props.label} ${props.value}`);
1630+
};
1631+
}, []);
1632+
16241633
return h.div({}, props.label, props.value);
16251634
});
16261635

@@ -1634,6 +1643,7 @@ describe(memo, () => {
16341643
h(Custom, { label: "y-hi", value: 0 })
16351644
)
16361645
);
1646+
await sleepFrame();
16371647
expect(parent).toMatchInlineSnapshot(`
16381648
<main>
16391649
<div>
@@ -1646,19 +1656,16 @@ describe(memo, () => {
16461656
</div>
16471657
</main>
16481658
`);
1649-
expect(mockFn.mock.calls).toMatchInlineSnapshot(`
1659+
expect(mockFnSnapshot()).toMatchInlineSnapshot(`
16501660
[
1651-
[
1652-
"x-hi",
1653-
0,
1654-
],
1655-
[
1656-
"y-hi",
1657-
0,
1658-
],
1661+
"[render] x-hi 0",
1662+
"[render] y-hi 0",
1663+
"[effect] x-hi 0",
1664+
"[effect] y-hi 0",
16591665
]
16601666
`);
16611667

1668+
mockFn.mockReset();
16621669
root.render(
16631670
h(
16641671
Fragment,
@@ -1667,6 +1674,7 @@ describe(memo, () => {
16671674
h(Custom, { label: "y-hi", value: 0 })
16681675
)
16691676
);
1677+
await sleepFrame();
16701678
expect(parent).toMatchInlineSnapshot(`
16711679
<main>
16721680
<div>
@@ -1679,19 +1687,9 @@ describe(memo, () => {
16791687
</div>
16801688
</main>
16811689
`);
1682-
expect(mockFn.mock.calls).toMatchInlineSnapshot(`
1683-
[
1684-
[
1685-
"x-hi",
1686-
0,
1687-
],
1688-
[
1689-
"y-hi",
1690-
0,
1691-
],
1692-
]
1693-
`);
1690+
expect(mockFnSnapshot()).toMatchInlineSnapshot("[]");
16941691

1692+
mockFn.mockReset();
16951693
root.render(
16961694
h(
16971695
Fragment,
@@ -1700,6 +1698,7 @@ describe(memo, () => {
17001698
h(Custom, { label: "y-hi", value: 0 })
17011699
)
17021700
);
1701+
await sleepFrame();
17031702
expect(parent).toMatchInlineSnapshot(`
17041703
<main>
17051704
<div>
@@ -1712,23 +1711,13 @@ describe(memo, () => {
17121711
</div>
17131712
</main>
17141713
`);
1715-
expect(mockFn.mock.calls).toMatchInlineSnapshot(`
1714+
expect(mockFnSnapshot()).toMatchInlineSnapshot(`
17161715
[
1717-
[
1718-
"x-hi",
1719-
0,
1720-
],
1721-
[
1722-
"y-hi",
1723-
0,
1724-
],
1725-
[
1726-
"x-hello",
1727-
0,
1728-
],
1716+
"[render] x-hello 0",
17291717
]
17301718
`);
17311719

1720+
mockFn.mockReset();
17321721
root.render(
17331722
h(
17341723
Fragment,
@@ -1737,6 +1726,7 @@ describe(memo, () => {
17371726
h(Custom, { label: "y-hi", value: 0 })
17381727
)
17391728
);
1729+
await sleepFrame();
17401730
expect(parent).toMatchInlineSnapshot(`
17411731
<main>
17421732
<div>
@@ -1749,27 +1739,13 @@ describe(memo, () => {
17491739
</div>
17501740
</main>
17511741
`);
1752-
expect(mockFn.mock.calls).toMatchInlineSnapshot(`
1742+
expect(mockFnSnapshot()).toMatchInlineSnapshot(`
17531743
[
1754-
[
1755-
"x-hi",
1756-
0,
1757-
],
1758-
[
1759-
"y-hi",
1760-
0,
1761-
],
1762-
[
1763-
"x-hello",
1764-
0,
1765-
],
1766-
[
1767-
"x-hi",
1768-
0,
1769-
],
1744+
"[render] x-hi 0",
17701745
]
17711746
`);
17721747

1748+
mockFn.mockReset();
17731749
root.render(
17741750
h(
17751751
Fragment,
@@ -1790,28 +1766,19 @@ describe(memo, () => {
17901766
</div>
17911767
</main>
17921768
`);
1793-
expect(mockFn.mock.calls).toMatchInlineSnapshot(`
1769+
expect(mockFnSnapshot()).toMatchInlineSnapshot(`
17941770
[
1795-
[
1796-
"x-hi",
1797-
0,
1798-
],
1799-
[
1800-
"y-hi",
1801-
0,
1802-
],
1803-
[
1804-
"x-hello",
1805-
0,
1806-
],
1807-
[
1808-
"x-hi",
1809-
0,
1810-
],
1811-
[
1812-
"x-hi",
1813-
1,
1814-
],
1771+
"[render] x-hi 1",
1772+
]
1773+
`);
1774+
1775+
mockFn.mockReset();
1776+
root.render(EMPTY_NODE);
1777+
expect(parent).toMatchInlineSnapshot("<main />");
1778+
expect(mockFnSnapshot()).toMatchInlineSnapshot(`
1779+
[
1780+
"[dispose] x-hi 0",
1781+
"[dispose] y-hi 0",
18151782
]
18161783
`);
18171784
});

0 commit comments

Comments
 (0)