Skip to content

Commit d7dd353

Browse files
fix(native): re-run an effect a batch notifies twice, and release a style entry by identity
A batch collected notified effects in a Set and iterated it, which skips a member added again after it ran. On a rotation, an entry that reads a viewport unit before a :root variable another entry shares runs before the variable recomputes, and the variable's notification then lands on a member already passed, so the element keeps the previous orientation's value. batch() deletes each effect before running it, so a second notification queues it again; the Dimensions listener and StyleCollection.inject both open their batch through it. A style entry deleted its hash from the cache once its last observer left, whatever entry the hash mapped to by then. React replays effects under StrictMode and when an Activity is shown again: the replay acquires a fresh entry for the hash and then releases the old one, which deleted the fresh entry. family.deleteIf deletes a key only while it maps to the given value, and an entry releases through it. The tests also cover the release itself: an entry goes when its last element unmounts, survives while shared, stays reactive while mounted, and the cache returns to empty after repeated mounts.
1 parent 8e16252 commit d7dd353

6 files changed

Lines changed: 492 additions & 67 deletions

File tree

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { Dimensions } from "react-native";
2+
3+
import { act, render, screen } from "@testing-library/react-native";
4+
import { View } from "react-native-css/components/View";
5+
import { registerCSS } from "react-native-css/jest";
6+
7+
import { batch, observable, type Effect } from "../../native/reactivity";
8+
9+
jest.mock("react-native", () => {
10+
const RN = jest.requireActual("react-native");
11+
RN.Platform.OS = "ios";
12+
return RN as unknown;
13+
});
14+
15+
const rotate = (width: number) => {
16+
act(() => {
17+
const metrics = { width, height: 800, scale: 2, fontScale: 1 };
18+
Dimensions.set({ window: metrics, screen: metrics });
19+
});
20+
};
21+
22+
const effectRecording = (record: () => void): Effect => ({
23+
observers: new Set(),
24+
run: record,
25+
});
26+
27+
test.each([
28+
["before", ".a { width: 10vw; color: var(--c); }"],
29+
["after", ".a { color: var(--c); width: 10vw; }"],
30+
])(
31+
"a rotation reaches an element reading a viewport unit %s a shared width-dependent variable",
32+
(_, rule) => {
33+
registerCSS(`
34+
:root { --c: blue; }
35+
@media (min-width: 500px) { :root { --c: red; } }
36+
${rule}
37+
.b { color: var(--c); }
38+
`);
39+
rotate(300);
40+
render(
41+
<>
42+
<View testID="a" className="a" />
43+
<View testID="b" className="b" />
44+
</>,
45+
);
46+
47+
for (const [width, color] of [
48+
[700, "red"],
49+
[300, "blue"],
50+
[700, "red"],
51+
] as const) {
52+
rotate(width);
53+
expect(screen.getByTestId("a")).toHaveStyle({ width: width / 10, color });
54+
expect(screen.getByTestId("b")).toHaveStyle({ color });
55+
}
56+
},
57+
);
58+
59+
test("an effect notified again while the batch drains runs again", () => {
60+
const source = observable(1);
61+
const derived = observable((read) => read(source) * 10);
62+
const seen: string[] = [];
63+
const subscriber = effectRecording(() => {
64+
seen.push(`${source.get()},${derived.get()}`);
65+
});
66+
67+
source.get(subscriber);
68+
derived.get(subscriber);
69+
batch(() => {
70+
source.set(2);
71+
});
72+
73+
expect(seen.at(-1)).toBe("2,20");
74+
});
75+
76+
test("a batch defers every notification until its update returns, and runs each effect once", () => {
77+
const width = observable(1);
78+
const height = observable(1);
79+
let runs = 0;
80+
const subscriber = effectRecording(() => {
81+
runs += 1;
82+
});
83+
84+
width.get(subscriber);
85+
height.get(subscriber);
86+
batch(() => {
87+
width.set(2);
88+
height.set(2);
89+
expect(runs).toBe(0);
90+
});
91+
92+
expect(runs).toBe(1);
93+
});

‎src/__tests__/native/reactivity-dependencies.test.tsx‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,35 @@ test("an explicit computed argument replaces its previous dependency", () => {
104104
cleanupEffect(subscriber);
105105
expect(second.observers.size).toBe(0);
106106
});
107+
108+
test("an observed computed value runs once per change, not once per read", () => {
109+
const source = observable(1);
110+
let computed = 0;
111+
const value = observable<number>((get) => {
112+
computed += 1;
113+
return get(source) * 2;
114+
});
115+
const subscriber: Effect = { observers: new Set(), run: jest.fn() };
116+
117+
for (let read = 0; read < 20; read += 1) {
118+
expect(value.get(subscriber)).toBe(2);
119+
}
120+
expect(computed).toBe(1);
121+
122+
source.set(5);
123+
expect(value.get(subscriber)).toBe(10);
124+
expect(computed).toBe(2);
125+
});
126+
127+
test("a family deletes a key by value only while the key still maps to it", () => {
128+
const cached = family((key: string) => ({ key }));
129+
const original = cached("shared");
130+
cached.delete("shared");
131+
const replacement = cached("shared");
132+
133+
expect(cached.deleteIf("shared", original)).toBe(false);
134+
expect(cached("shared")).toBe(replacement);
135+
136+
expect(cached.deleteIf("shared", replacement)).toBe(true);
137+
expect(cached.size()).toBe(0);
138+
});

0 commit comments

Comments
 (0)