Skip to content

Commit 8f15dc8

Browse files
welkinwongitalojs
welkinwong
authored and
italojs
committed
fix: #443 useSyncEffect Breaks useFind Reactivity Under Suspense
In the client-side implementation of suspense/useFind, useFindClient is used. PR #433 introduced useSyncEffect. Nevertheless, when used in Strict Mode and with Suspense, the useSyncEffect implementation results in non-trivial and hard-to-predict issues. As a result, the Suspense-enabled useFindSuspenseClient will no longer reference useFindClient and will instead be a standalone implementation (derived from the last stable version of useFindClient).
1 parent 269867b commit 8f15dc8

File tree

2 files changed

+267
-65
lines changed

2 files changed

+267
-65
lines changed

‎packages/react-meteor-data/suspense/useFind.tests.js

Lines changed: 105 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,153 @@
11
/* global Meteor, Tinytest */
2-
import React, { Suspense } from 'react'
3-
import { renderToString } from 'react-dom/server'
4-
import { Mongo } from 'meteor/mongo'
2+
import React, { Suspense } from 'react';
3+
import { renderToString } from 'react-dom/server';
4+
import { Mongo } from 'meteor/mongo';
5+
import { renderHook } from '@testing-library/react';
6+
import { useFindSuspenseClient, useFindSuspenseServer } from './useFind';
7+
8+
/**
9+
* Test for useFindSuspenseClient
10+
*/
11+
if (Meteor.isClient) {
12+
Tinytest.addAsync(
13+
'suspense/useFindSuspenseClient - Verify reference stability between rerenders',
14+
async (test) => {
15+
const TestDocs = new Mongo.Collection(null);
16+
17+
TestDocs.insert({ id: 0, updated: 0 });
18+
TestDocs.insert({ id: 1, updated: 0 });
19+
20+
const { result, rerender } = renderHook(() =>
21+
useFindSuspenseClient(TestDocs, [{}])
22+
);
23+
24+
test.equal(
25+
result.current.length,
26+
2,
27+
'2 items should have rendered, only 2, no more.'
28+
);
29+
30+
await TestDocs.updateAsync({ id: 1 }, { $inc: { updated: 1 } });
31+
32+
rerender();
33+
34+
test.equal(
35+
result.current.length,
36+
2,
37+
'2 items should have rendered - only 1 of the items should have been matched by the reconciler after a single change.'
38+
);
39+
}
40+
);
541

6-
import { useFindSuspense } from './useFind'
42+
Tinytest.addAsync(
43+
'suspense/useFindSuspenseClient - null return is allowed',
44+
async (test) => {
45+
const TestDocs = new Mongo.Collection(null);
46+
47+
TestDocs.insertAsync({ id: 0, updated: 0 });
48+
49+
const { result } = renderHook(() =>
50+
useFindSuspenseClient(TestDocs, null)
51+
);
752

53+
test.isNull(
54+
result.current,
55+
'Return value should be null when the factory returns null'
56+
);
57+
}
58+
);
59+
}
60+
61+
/**
62+
* Test for useFindSuspenseServer
63+
*/
864
if (Meteor.isServer) {
965
Tinytest.addAsync(
10-
'suspense/useFind - Data query validation',
66+
'suspense/useFindSuspenseServer - Data query validation',
1167
async function (test) {
12-
const TestDocs = new Mongo.Collection(null)
68+
const TestDocs = new Mongo.Collection(null);
1369

14-
TestDocs.insertAsync({ id: 0, updated: 0 })
70+
TestDocs.insertAsync({ id: 0, updated: 0 });
1571

16-
let returnValue
72+
let returnValue;
1773

1874
const Test = () => {
19-
returnValue = useFindSuspense(TestDocs, [{}])
75+
returnValue = useFindSuspenseServer(TestDocs, [{}]);
2076

21-
return null
22-
}
77+
return null;
78+
};
2379
const TestSuspense = () => {
2480
return (
2581
<Suspense fallback={<div>Loading...</div>}>
2682
<Test />
2783
</Suspense>
28-
)
29-
}
84+
);
85+
};
3086

3187
// first return promise
32-
renderToString(<TestSuspense />)
88+
renderToString(<TestSuspense />);
3389
test.isUndefined(
3490
returnValue,
3591
'Return value should be undefined as find promise unresolved'
3692
);
3793
// wait promise
38-
await new Promise((resolve) => setTimeout(resolve, 100))
94+
await new Promise((resolve) => setTimeout(resolve, 100));
3995
// return data
40-
renderToString(<TestSuspense />)
96+
renderToString(<TestSuspense />);
4197

4298
test.equal(
4399
returnValue.length,
44100
1,
45101
'Return value should be an array with one document'
46-
)
102+
);
47103
}
48-
)
104+
);
49105

50106
Tinytest.addAsync(
51-
'suspense/useFind - Test proper cache invalidation',
107+
'suspense/useFindSuspenseServer - Test proper cache invalidation',
52108
async function (test) {
53-
const TestDocs = new Mongo.Collection(null)
109+
const TestDocs = new Mongo.Collection(null);
54110

55-
TestDocs.insertAsync({ id: 0, updated: 0 })
111+
TestDocs.insertAsync({ id: 0, updated: 0 });
56112

57-
let returnValue
113+
let returnValue;
58114

59115
const Test = () => {
60-
returnValue = useFindSuspense(TestDocs, [{}])
116+
returnValue = useFindSuspenseServer(TestDocs, [{}]);
61117

62-
return null
63-
}
118+
return null;
119+
};
64120
const TestSuspense = () => {
65121
return (
66122
<Suspense fallback={<div>Loading...</div>}>
67123
<Test />
68124
</Suspense>
69-
)
70-
}
125+
);
126+
};
71127

72128
// first return promise
73-
renderToString(<TestSuspense />)
129+
renderToString(<TestSuspense />);
74130

75131
test.isUndefined(
76132
returnValue,
77133
'Return value should be undefined as find promise unresolved'
78134
);
79135
// wait promise
80-
await new Promise((resolve) => setTimeout(resolve, 100))
136+
await new Promise((resolve) => setTimeout(resolve, 100));
81137
// return data
82-
renderToString(<TestSuspense />)
138+
renderToString(<TestSuspense />);
83139

84140
test.equal(
85141
returnValue[0].updated,
86142
0,
87143
'Return value should be an array with initial value as find promise resolved'
88144
);
89145

90-
TestDocs.updateAsync({ id: 0 }, { $inc: { updated: 1 } })
91-
await new Promise((resolve) => setTimeout(resolve, 100))
146+
TestDocs.updateAsync({ id: 0 }, { $inc: { updated: 1 } });
147+
await new Promise((resolve) => setTimeout(resolve, 100));
92148

93149
// second return promise
94-
renderToString(<TestSuspense />)
150+
renderToString(<TestSuspense />);
95151

96152
test.equal(
97153
returnValue[0].updated,
@@ -100,46 +156,46 @@ if (Meteor.isServer) {
100156
);
101157

102158
// wait promise
103-
await new Promise((resolve) => setTimeout(resolve, 100))
159+
await new Promise((resolve) => setTimeout(resolve, 100));
104160
// return data
105-
renderToString(<TestSuspense />)
161+
renderToString(<TestSuspense />);
106162

107163
test.equal(
108164
returnValue[0].updated,
109165
1,
110166
'Return value should be an array with one document with value updated'
111-
)
167+
);
112168
}
113-
)
169+
);
114170

115171
Tinytest.addAsync(
116-
'suspense/useFind - null return is allowed',
172+
'suspense/useFindSuspenseServer - null return is allowed',
117173
async function (test) {
118-
const TestDocs = new Mongo.Collection(null)
174+
const TestDocs = new Mongo.Collection(null);
119175

120-
TestDocs.insertAsync({ id: 0, updated: 0 })
176+
TestDocs.insertAsync({ id: 0, updated: 0 });
121177

122-
let returnValue
178+
let returnValue;
123179

124180
const Test = () => {
125-
returnValue = useFindSuspense(TestDocs, null)
181+
returnValue = useFindSuspenseServer(TestDocs, null);
126182

127-
return null
128-
}
183+
return null;
184+
};
129185
const TestSuspense = () => {
130186
return (
131187
<Suspense fallback={<div>Loading...</div>}>
132188
<Test />
133189
</Suspense>
134-
)
135-
}
190+
);
191+
};
136192

137-
renderToString(<TestSuspense returnNull={true} />)
193+
renderToString(<TestSuspense returnNull={true} />);
138194

139195
test.isNull(
140196
returnValue,
141197
'Return value should be null when the factory returns null'
142-
)
198+
);
143199
}
144-
)
200+
);
145201
}

0 commit comments

Comments
 (0)