Skip to content

Commit

Permalink
fix(batch): run event-triggered changes immediately
Browse files Browse the repository at this point in the history
When react-easy-state changes are triggered from the main event loop,
they must run inside the main event loop, instead of being batched in
the microtask. Otherwise, the cursor will jump to the end of input
elements, since the change can't be tied back to the action
taken. React batches changes caused inside an event loop, so we can
rely on the normal React setState batching.
  • Loading branch information
justinweiss committed Apr 5, 2022
1 parent f3038a2 commit 31e541c
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 9 deletions.
8 changes: 0 additions & 8 deletions __tests__/batching.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,6 @@ describe('batching', () => {

expect(container).toHaveTextContent('2');
expect(renderCount).toBe(2);

easyAct(() => {
fireEvent.click(button);
fireEvent.click(button);
});

expect(container).toHaveTextContent('6');
expect(renderCount).toBe(3);
});

// TODO: batching native event handlers causes in input caret jumping bug
Expand Down
77 changes: 76 additions & 1 deletion src/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
isObservable,
} from '@nx-js/observer-util';

import { hasHooks } from './utils';
import { globalObj, hasHooks } from './utils';

export let isInsideFunctionComponent = false;
export let isInsideClassComponentRender = false;
Expand All @@ -32,10 +32,25 @@ function mapStateToStores(state) {
// is to prevent excessive rendering in situations where updates can occur
// outside of React's built-in batching. e.g. after resolving a promise,
// in a setTimeout callback, in an event handler.
//
// NOTE: This should be revisited after React improves batching for
// Suspense / etc.
let batchesPending = {};
let taskPending = false;
let viewIndexCounter = 0;
let inEventLoop = false;

function batchSetState(viewIndex, fn) {
if (inEventLoop) {
// If we are in the main event loop, React handles the batching
// automatically, so we run the change immediately. Deferring the
// update can cause unexpected cursor shifts in input elements,
// since the change can't be tied back to the action:
// https://github.com/facebook/react/issues/5386
fn();
return;
}

batchesPending[viewIndex] = fn;
if (!taskPending) {
taskPending = true;
Expand All @@ -57,6 +72,66 @@ function clearBatch(viewIndex) {
delete batchesPending[viewIndex];
}

// this creates and returns a wrapped version of the passed function
// the cache is necessary to always map the same thing to the same function
// which makes sure that addEventListener/removeEventListener pairs don't break
const cache = new WeakMap();
function wrapFn(fn, wrapper) {
if (typeof fn !== 'function') {
return fn;
}
let wrapped = cache.get(fn);
if (!wrapped) {
wrapped = function(...args) {
return wrapper(fn, this, args);
};
cache.set(fn, wrapped);
}
return wrapped;
}

function wrapMethodCallbacks(obj, method, wrapper) {
const descriptor = Object.getOwnPropertyDescriptor(obj, method);
if (
descriptor &&
descriptor.writable &&
typeof descriptor.value === 'function'
) {
obj[method] = new Proxy(descriptor.value, {
apply(target, ctx, args) {
return Reflect.apply(
target,
ctx,
args.map(f => wrapFn(f, wrapper)),
);
},
});
}
}

// wrapped obj.addEventListener(cb) like callbacks
function wrapMethodsCallbacks(obj, methods, wrapper) {
methods.forEach(method =>
wrapMethodCallbacks(obj, method, wrapper),
);
}

// batch addEventListener calls
if (globalObj.EventTarget) {
wrapMethodsCallbacks(
EventTarget.prototype,
['addEventListener', 'removeEventListener'],
(fn, ctx, args) => {
inEventLoop = true;
try {
fn.apply(ctx, args);
} finally {
inEventLoop = false;
}
},
);
}

export function view(Comp) {
const isStatelessComp = !(
Comp.prototype && Comp.prototype.isReactComponent
Expand Down

0 comments on commit 31e541c

Please sign in to comment.