Skip to content

Commit 0163659

Browse files
solkimicrebsolkimicreb
committed
fix(scheduler): fix render order
1 parent 800e8ed commit 0163659

17 files changed

+309
-239
lines changed

‎__mocks__/react-platform.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = process.env.NATIVE
2+
? require('react-native')
3+
: require('react-dom');

‎__tests__/batching.test.jsx‎

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -131,28 +131,55 @@ describe('batching', () => {
131131
expect(renderCount).toBe(3);
132132
});
133133

134-
test('should work with nested batch() calls', () => {
135-
let renderCount = 0;
136-
const person = store({ name: 'Bob' });
137-
const MyComp = view(() => {
138-
renderCount += 1;
139-
return <div>{person.name}</div>;
134+
describe('nested batch()', () => {
135+
test('should work with renders', () => {
136+
let renderCount = 0;
137+
const person = store({ name: 'Bob' });
138+
const MyComp = view(() => {
139+
renderCount += 1;
140+
return <div>{person.name}</div>;
141+
});
142+
143+
const { container } = render(<MyComp />);
144+
expect(renderCount).toBe(1);
145+
expect(container).toHaveTextContent('Bob');
146+
batch(() => {
147+
batch(() => {
148+
person.name = 'Rob';
149+
person.name = 'David';
150+
});
151+
expect(container).toHaveTextContent('Bob');
152+
person.name = 'Ann';
153+
person.name = 'Rick';
154+
});
155+
expect(container).toHaveTextContent('Rick');
156+
expect(renderCount).toBe(2);
140157
});
141158

142-
const { container } = render(<MyComp />);
143-
expect(renderCount).toBe(1);
144-
expect(container).toHaveTextContent('Bob');
145-
batch(() => {
159+
test('should work with autoEffect', () => {
160+
let name;
161+
let numOfRuns = 0;
162+
const person = store({ name: 'Bob' });
163+
164+
const effect = autoEffect(() => {
165+
numOfRuns += 1;
166+
name = person.name;
167+
});
168+
146169
batch(() => {
147-
person.name = 'Rob';
148-
person.name = 'David';
170+
batch(() => {
171+
person.name = 'Rob';
172+
person.name = 'David';
173+
});
174+
expect(name).toBe('Bob');
175+
person.name = 'Ann';
176+
person.name = 'Rick';
149177
});
150-
expect(container).toHaveTextContent('Bob');
151-
person.name = 'Ann';
152-
person.name = 'Rick';
178+
expect(name).toBe('Rick');
179+
expect(numOfRuns).toBe(2);
180+
181+
clearEffect(effect);
153182
});
154-
expect(container).toHaveTextContent('Rick');
155-
expect(renderCount).toBe(2);
156183
});
157184

158185
test('should recover when error thrown inside batch', () => {

‎__tests__/edgeCases.test.jsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,7 @@ describe('edge cases', () => {
182182
expect(container).toHaveTextContent('1');
183183
});
184184

185-
// TODO: fix zombie updates
186-
describe.skip('reactive renders should run in parent - child order with no duplicate child runs from props', () => {
185+
describe('reactive renders should run in parent - child order with no duplicate child runs from props', () => {
187186
test('should work with function components', () => {
188187
const appStore = store({ num: 1, nested: { num: 12 } });
189188

‎package-lock.json‎

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@
110110
"rollup": "^2.0.6",
111111
"rollup-plugin-auto-external": "^2.0.0",
112112
"rollup-plugin-babel": "^4.4.0",
113+
"rollup-plugin-replace": "^2.2.0",
113114
"semantic-release": "^17.0.4",
114115
"sinon": "^9.0.1",
115116
"styled-components": "^5.0.1"

‎rollup.config.js‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,32 @@
1-
import path from 'path';
2-
import resolvePlugin from '@rollup/plugin-node-resolve';
3-
import babelPlugin from 'rollup-plugin-babel';
4-
import externalsPlugin from 'rollup-plugin-auto-external';
1+
const path = require('path');
2+
const replacePlugin = require('rollup-plugin-replace');
3+
const resolvePlugin = require('@rollup/plugin-node-resolve');
4+
const babelPlugin = require('rollup-plugin-babel');
5+
const externalsPlugin = require('rollup-plugin-auto-external');
56

6-
export default [
7+
// this is also used in watch mode by the startExample script
8+
const defaultBuild = [
9+
{
10+
input: path.resolve('src/platforms/dom.js'),
11+
external: ['react-dom'],
12+
output: [
13+
{
14+
format: 'es',
15+
dir: 'dist',
16+
entryFileNames: 'react-platform.js',
17+
},
18+
{
19+
format: 'cjs',
20+
dir: 'dist',
21+
entryFileNames: 'react-platform.cjs.js',
22+
},
23+
],
24+
},
725
{
826
input: path.resolve('src/index.js'),
27+
external: ['./react-platform'],
928
plugins: [
29+
replacePlugin({ 'react-platform': './react-platform' }),
1030
resolvePlugin(),
1131
babelPlugin({ exclude: 'node_modules/**' }),
1232
externalsPlugin({ dependencies: true, peerDependecies: true }),
@@ -18,9 +38,31 @@ export default [
1838
sourcemap: true,
1939
},
2040
},
41+
];
42+
43+
const allBuilds = [
44+
...defaultBuild,
45+
{
46+
input: path.resolve('src/platforms/native.js'),
47+
external: ['react-native'],
48+
output: [
49+
{
50+
format: 'es',
51+
dir: 'dist',
52+
entryFileNames: 'react-platform.native.js',
53+
},
54+
{
55+
format: 'cjs',
56+
dir: 'dist',
57+
entryFileNames: 'react-platform.cjs.native.js',
58+
},
59+
],
60+
},
2161
{
2262
input: path.resolve('src/index.js'),
63+
external: ['./react-platform'],
2364
plugins: [
65+
replacePlugin({ 'react-platform': './react-platform' }),
2466
resolvePlugin(),
2567
babelPlugin({
2668
exclude: 'node_modules/**',
@@ -37,7 +79,9 @@ export default [
3779
},
3880
{
3981
input: path.resolve('src/index.js'),
82+
external: ['./react-platform.cjs'],
4083
plugins: [
84+
replacePlugin({ 'react-platform': './react-platform.cjs' }),
4185
resolvePlugin(),
4286
babelPlugin({ exclude: 'node_modules/**' }),
4387
externalsPlugin({ dependencies: true, peerDependecies: true }),
@@ -51,7 +95,9 @@ export default [
5195
},
5296
{
5397
input: path.resolve('src/index.js'),
98+
external: ['./react-platform.cjs'],
5499
plugins: [
100+
replacePlugin({ 'react-platform': './react-platform.cjs' }),
55101
resolvePlugin(),
56102
babelPlugin({
57103
exclude: 'node_modules/**',
@@ -67,3 +113,9 @@ export default [
67113
},
68114
},
69115
];
116+
117+
module.exports = {
118+
defaultBuild,
119+
// this has to be exported as default for rollup CLI to pick it up
120+
default: allBuilds,
121+
};

‎scripts/startExample.js‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ const path = require('path');
33
const fs = require('fs');
44
const { execSync: exec } = require('child_process');
55
const rollup = require('rollup');
6-
const resolvePlugin = require('@rollup/plugin-node-resolve');
7-
const babelPlugin = require('rollup-plugin-babel');
8-
const externalsPlugin = require('rollup-plugin-auto-external');
6+
const { defaultBuild } = require('../rollup.config');
97

108
console.customInfo = args => {
119
console.info(
@@ -36,24 +34,14 @@ if (!fs.existsSync(exampleFolder)) {
3634

3735
// 1. BUILD EASY-STATE BUNDLE
3836
console.customInfo('Building react-easy-state in watch mode.');
39-
const watchOptions = {
40-
input: path.resolve('src/index.js'),
41-
plugins: [
42-
resolvePlugin(),
43-
babelPlugin({ exclude: 'node_modules/**' }),
44-
externalsPlugin({ dependencies: true, peerDependecies: true }),
45-
],
46-
output: {
47-
format: 'es',
48-
dir: 'dist',
49-
entryFileNames: 'es.es6.js',
50-
sourcemap: true,
51-
},
52-
watch: {
53-
include: 'src/**',
54-
},
55-
};
56-
const watcher = rollup.watch(watchOptions);
37+
const watcher = rollup.watch(
38+
defaultBuild.map(config => ({
39+
...config,
40+
watch: {
41+
include: 'src/**',
42+
},
43+
})),
44+
);
5745
watcher.on('event', ({ code }) => {
5846
if (code === 'START') {
5947
console.customInfo('Building bundle.');

‎scripts/testBuilds.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ const { exec } = require('child_process');
66
const distPath = path.resolve('dist');
77
const files = fs
88
.readdirSync(distPath)
9-
.filter(dist => dist.indexOf('map') === -1);
9+
.filter(
10+
dist =>
11+
dist.indexOf('map') === -1 && dist.indexOf('platform') === -1,
12+
);
1013

1114
function execPromise(cmd) {
1215
return new Promise(resolve => exec(cmd, resolve));

‎src/autoEffect.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useEffect } from 'react';
22
import { observe, unobserve } from '@nx-js/observer-util';
3-
import { queue } from './queue';
3+
import scheduler from './scheduler';
44

55
import {
66
isInsideFunctionComponent,
@@ -12,7 +12,7 @@ export function autoEffect(fn, deps = []) {
1212
if (isInsideFunctionComponent) {
1313
return useEffect(() => {
1414
const observer = observe(fn, {
15-
scheduler: () => queue.add(observer),
15+
scheduler: () => scheduler.add(observer),
1616
});
1717
return () => unobserve(observer);
1818
}, deps);
@@ -29,7 +29,7 @@ export function autoEffect(fn, deps = []) {
2929
}
3030

3131
const observer = observe(fn, {
32-
scheduler: () => queue.add(observer),
32+
scheduler: () => scheduler.add(observer),
3333
});
3434
return observer;
3535
}

0 commit comments

Comments
 (0)