Skip to content

Commit

Permalink
fix(scheduler): fix render order
Browse files Browse the repository at this point in the history
  • Loading branch information
solkimicreb committed Apr 24, 2020
1 parent 800e8ed commit 0163659
Show file tree
Hide file tree
Showing 17 changed files with 309 additions and 239 deletions.
3 changes: 3 additions & 0 deletions __mocks__/react-platform.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = process.env.NATIVE
? require('react-native')
: require('react-dom');
61 changes: 44 additions & 17 deletions __tests__/batching.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,28 +131,55 @@ describe('batching', () => {
expect(renderCount).toBe(3);
});

test('should work with nested batch() calls', () => {
let renderCount = 0;
const person = store({ name: 'Bob' });
const MyComp = view(() => {
renderCount += 1;
return <div>{person.name}</div>;
describe('nested batch()', () => {
test('should work with renders', () => {
let renderCount = 0;
const person = store({ name: 'Bob' });
const MyComp = view(() => {
renderCount += 1;
return <div>{person.name}</div>;
});

const { container } = render(<MyComp />);
expect(renderCount).toBe(1);
expect(container).toHaveTextContent('Bob');
batch(() => {
batch(() => {
person.name = 'Rob';
person.name = 'David';
});
expect(container).toHaveTextContent('Bob');
person.name = 'Ann';
person.name = 'Rick';
});
expect(container).toHaveTextContent('Rick');
expect(renderCount).toBe(2);
});

const { container } = render(<MyComp />);
expect(renderCount).toBe(1);
expect(container).toHaveTextContent('Bob');
batch(() => {
test('should work with autoEffect', () => {
let name;
let numOfRuns = 0;
const person = store({ name: 'Bob' });

const effect = autoEffect(() => {
numOfRuns += 1;
name = person.name;
});

batch(() => {
person.name = 'Rob';
person.name = 'David';
batch(() => {
person.name = 'Rob';
person.name = 'David';
});
expect(name).toBe('Bob');
person.name = 'Ann';
person.name = 'Rick';
});
expect(container).toHaveTextContent('Bob');
person.name = 'Ann';
person.name = 'Rick';
expect(name).toBe('Rick');
expect(numOfRuns).toBe(2);

clearEffect(effect);
});
expect(container).toHaveTextContent('Rick');
expect(renderCount).toBe(2);
});

test('should recover when error thrown inside batch', () => {
Expand Down
3 changes: 1 addition & 2 deletions __tests__/edgeCases.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ describe('edge cases', () => {
expect(container).toHaveTextContent('1');
});

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

Expand Down
25 changes: 25 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"rollup": "^2.0.6",
"rollup-plugin-auto-external": "^2.0.0",
"rollup-plugin-babel": "^4.4.0",
"rollup-plugin-replace": "^2.2.0",
"semantic-release": "^17.0.4",
"sinon": "^9.0.1",
"styled-components": "^5.0.1"
Expand Down
62 changes: 57 additions & 5 deletions rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,32 @@
import path from 'path';
import resolvePlugin from '@rollup/plugin-node-resolve';
import babelPlugin from 'rollup-plugin-babel';
import externalsPlugin from 'rollup-plugin-auto-external';
const path = require('path');
const replacePlugin = require('rollup-plugin-replace');
const resolvePlugin = require('@rollup/plugin-node-resolve');
const babelPlugin = require('rollup-plugin-babel');
const externalsPlugin = require('rollup-plugin-auto-external');

export default [
// this is also used in watch mode by the startExample script
const defaultBuild = [
{
input: path.resolve('src/platforms/dom.js'),
external: ['react-dom'],
output: [
{
format: 'es',
dir: 'dist',
entryFileNames: 'react-platform.js',
},
{
format: 'cjs',
dir: 'dist',
entryFileNames: 'react-platform.cjs.js',
},
],
},
{
input: path.resolve('src/index.js'),
external: ['./react-platform'],
plugins: [
replacePlugin({ 'react-platform': './react-platform' }),
resolvePlugin(),
babelPlugin({ exclude: 'node_modules/**' }),
externalsPlugin({ dependencies: true, peerDependecies: true }),
Expand All @@ -18,9 +38,31 @@ export default [
sourcemap: true,
},
},
];

const allBuilds = [
...defaultBuild,
{
input: path.resolve('src/platforms/native.js'),
external: ['react-native'],
output: [
{
format: 'es',
dir: 'dist',
entryFileNames: 'react-platform.native.js',
},
{
format: 'cjs',
dir: 'dist',
entryFileNames: 'react-platform.cjs.native.js',
},
],
},
{
input: path.resolve('src/index.js'),
external: ['./react-platform'],
plugins: [
replacePlugin({ 'react-platform': './react-platform' }),
resolvePlugin(),
babelPlugin({
exclude: 'node_modules/**',
Expand All @@ -37,7 +79,9 @@ export default [
},
{
input: path.resolve('src/index.js'),
external: ['./react-platform.cjs'],
plugins: [
replacePlugin({ 'react-platform': './react-platform.cjs' }),
resolvePlugin(),
babelPlugin({ exclude: 'node_modules/**' }),
externalsPlugin({ dependencies: true, peerDependecies: true }),
Expand All @@ -51,7 +95,9 @@ export default [
},
{
input: path.resolve('src/index.js'),
external: ['./react-platform.cjs'],
plugins: [
replacePlugin({ 'react-platform': './react-platform.cjs' }),
resolvePlugin(),
babelPlugin({
exclude: 'node_modules/**',
Expand All @@ -67,3 +113,9 @@ export default [
},
},
];

module.exports = {
defaultBuild,
// this has to be exported as default for rollup CLI to pick it up
default: allBuilds,
};
30 changes: 9 additions & 21 deletions scripts/startExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ const path = require('path');
const fs = require('fs');
const { execSync: exec } = require('child_process');
const rollup = require('rollup');
const resolvePlugin = require('@rollup/plugin-node-resolve');
const babelPlugin = require('rollup-plugin-babel');
const externalsPlugin = require('rollup-plugin-auto-external');
const { defaultBuild } = require('../rollup.config');

console.customInfo = args => {
console.info(
Expand Down Expand Up @@ -36,24 +34,14 @@ if (!fs.existsSync(exampleFolder)) {

// 1. BUILD EASY-STATE BUNDLE
console.customInfo('Building react-easy-state in watch mode.');
const watchOptions = {
input: path.resolve('src/index.js'),
plugins: [
resolvePlugin(),
babelPlugin({ exclude: 'node_modules/**' }),
externalsPlugin({ dependencies: true, peerDependecies: true }),
],
output: {
format: 'es',
dir: 'dist',
entryFileNames: 'es.es6.js',
sourcemap: true,
},
watch: {
include: 'src/**',
},
};
const watcher = rollup.watch(watchOptions);
const watcher = rollup.watch(
defaultBuild.map(config => ({
...config,
watch: {
include: 'src/**',
},
})),
);
watcher.on('event', ({ code }) => {
if (code === 'START') {
console.customInfo('Building bundle.');
Expand Down
5 changes: 4 additions & 1 deletion scripts/testBuilds.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ const { exec } = require('child_process');
const distPath = path.resolve('dist');
const files = fs
.readdirSync(distPath)
.filter(dist => dist.indexOf('map') === -1);
.filter(
dist =>
dist.indexOf('map') === -1 && dist.indexOf('platform') === -1,
);

function execPromise(cmd) {
return new Promise(resolve => exec(cmd, resolve));
Expand Down
6 changes: 3 additions & 3 deletions src/autoEffect.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useEffect } from 'react';
import { observe, unobserve } from '@nx-js/observer-util';
import { queue } from './queue';
import scheduler from './scheduler';

import {
isInsideFunctionComponent,
Expand All @@ -12,7 +12,7 @@ export function autoEffect(fn, deps = []) {
if (isInsideFunctionComponent) {
return useEffect(() => {
const observer = observe(fn, {
scheduler: () => queue.add(observer),
scheduler: () => scheduler.add(observer),
});
return () => unobserve(observer);
}, deps);
Expand All @@ -29,7 +29,7 @@ export function autoEffect(fn, deps = []) {
}

const observer = observe(fn, {
scheduler: () => queue.add(observer),
scheduler: () => scheduler.add(observer),
});
return observer;
}
Expand Down
Loading

0 comments on commit 0163659

Please sign in to comment.