Skip to content

Commit

Permalink
feat(store): auto batch store methods
Browse files Browse the repository at this point in the history
  • Loading branch information
solkimicreb committed Apr 24, 2020
1 parent 0bc26e5 commit 3cc190e
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 10 deletions.
106 changes: 101 additions & 5 deletions __tests__/batching.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import React, { Component } from 'react';
import { render, cleanup, act } from '@testing-library/react/pure';
import {
render,
cleanup,
fireEvent,
act,
} from '@testing-library/react/pure';
import {
view,
store,
Expand Down Expand Up @@ -187,7 +192,35 @@ describe('batching', () => {
expect(renderCount).toBe(2);
});

test('should batch state changes inside native event listeners', () => {
test('should batch state changes in react event listeners', () => {
let renderCount = 0;
const counter = store({ num: 0 });
const MyComp = view(() => {
renderCount += 1;
return (
<button
type="button"
onClick={() => {
counter.num += 1;
counter.num += 1;
}}
>
{counter.num}
</button>
);
});

const { container } = render(<MyComp />);
const button = container.querySelector('button');
expect(renderCount).toBe(1);
expect(container).toHaveTextContent('0');
fireEvent.click(button);
expect(container).toHaveTextContent('2');
expect(renderCount).toBe(2);
});

// TODO: batching native event handlers causes in input caret jumping bug
test.skip('should batch state changes inside native event listeners', () => {
let renderCount = 0;
const person = store({ name: 'Bob' });
const MyComp = view(() => {
Expand All @@ -198,15 +231,15 @@ describe('batching', () => {
const { container } = render(<MyComp />);
expect(renderCount).toBe(1);
expect(container).toHaveTextContent('Bob');
const batched = act(() => {
const handler = act(() => {
person.name = 'Ann';
person.name = 'Rick';
});
document.body.addEventListener('click', batched);
document.body.addEventListener('click', handler);
document.body.dispatchEvent(new Event('click'));
expect(container).toHaveTextContent('Rick');
expect(renderCount).toBe(2);
document.body.removeEventListener('click', batched);
document.body.removeEventListener('click', handler);
});

// async/await is only batched when it is transpiled to promises and/or generators
Expand Down Expand Up @@ -314,6 +347,69 @@ describe('batching', () => {
expect(renderCount).toBe(2);
});

test('should batch changes in store methods', () => {
let numOfRuns = 0;
let name = '';

const myStore = store({
firstName: 'My',
lastName: 'Store',
setName(firstName, lastName) {
this.firstName = firstName;
this.lastName = lastName;
},
});

const effect = autoEffect(() => {
name = `${myStore.firstName} ${myStore.lastName}`;
numOfRuns += 1;
});
expect(name).toBe('My Store');
expect(numOfRuns).toBe(1);

myStore.setName('Awesome', 'Stuff');
expect(name).toBe('Awesome Stuff');
expect(numOfRuns).toBe(2);

clearEffect(effect);
});

test('should batch changes in store setters', () => {
let numOfRuns = 0;
let name = '';

const myStore = store({
firstName: 'My',
middleName: 'Little',
lastName: 'Store',
get name() {
return `${this.firstName} ${this.middleName} ${this.lastName}`;
},
set name(newName) {
const [firstName, middleName, lastName] = newName.split(' ');
this.firstName = firstName;
this.middleName = middleName;
this.lastName = lastName;
},
});

const effect = autoEffect(() => {
name = myStore.name;
numOfRuns += 1;
});
expect(name).toBe('My Little Store');
expect(numOfRuns).toBe(1);

myStore.name = 'Awesome JS Stuff';
expect(name).toBe('Awesome JS Stuff');
expect(myStore.name).toBe('Awesome JS Stuff');
// the reactions runs two times once for the setter call
// and once for all the mutations inside the setter (alltogether)
expect(numOfRuns).toBe(3);

clearEffect(effect);
});

test('should not break Promises', async () => {
await Promise.resolve(12)
.then(value => {
Expand Down
38 changes: 33 additions & 5 deletions src/store.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,43 @@
import { useMemo } from 'react';
import { observable } from '@nx-js/observer-util';

import { batch } from './scheduler';
import {
isInsideFunctionComponent,
isInsideClassComponentRender,
isInsideFunctionComponentWithoutHooks,
} from './view';

function batchMethods(obj) {
Object.getOwnPropertyNames(obj).forEach(key => {
const { value, set } = Object.getOwnPropertyDescriptor(obj, key);

// batch store methods
if (typeof value === 'function') {
// use a Proxy instead of function wrapper to keep the method name
obj[key] = new Proxy(value, {
apply(target, thisArg, args) {
return batch(target, thisArg, args);
},
});
} else if (set) {
// batch property setters
Object.defineProperty(obj, key, {
set(newValue) {
return batch(set, obj, [newValue]);
},
});
}
});
return obj;
}

function createStore(obj) {
return batchMethods(
observable(typeof obj === 'function' ? obj() : obj),
);
}

export function store(obj) {
// do not create new versions of the store on every render
// if it is a local store in a function component
Expand All @@ -15,10 +46,7 @@ export function store(obj) {
// useMemo is not a semantic guarantee
// In the future, React may choose to “forget” some previously memoized values and recalculate them on next render
// see this docs for more explanation: https://reactjs.org/docs/hooks-reference.html#usememo
return useMemo(
() => observable(typeof obj === 'function' ? obj() : obj),
[],
);
return useMemo(() => createStore(obj), []);
}
if (isInsideFunctionComponentWithoutHooks) {
throw new Error(
Expand All @@ -30,5 +58,5 @@ export function store(obj) {
'You cannot use state inside a render of a class component. Please create your store outside of the render function.',
);
}
return observable(typeof obj === 'function' ? obj() : obj);
return createStore(obj);
}

0 comments on commit 3cc190e

Please sign in to comment.