Skip to content

Commit

Permalink
fix: do not skip all updates if the rendering take long time (#664)
Browse files Browse the repository at this point in the history
  • Loading branch information
T-Wizard committed Sep 1, 2021
1 parent 998b2fe commit 4811d14
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 14 deletions.
21 changes: 7 additions & 14 deletions apis/nucleus/src/components/Supernova.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useState, useEffect, useCallback, useContext, useRef } from 'react';
import React, { useState, useEffect, useCallback, useContext } from 'react';
import InstanceContext from '../contexts/InstanceContext';
import useRect from '../hooks/useRect';
import RenderDebouncer from '../utils/render-debouncer';

/**
* @interface
Expand All @@ -16,7 +17,7 @@ const Supernova = ({ sn, snOptions: options, snPlugins: plugins, layout, appLayo
const { component } = sn;

const { theme: themeName, language, constraints } = useContext(InstanceContext);
const renderDebouncer = useRef(null);
const [renderDebouncer] = useState(() => new RenderDebouncer());
const [isMounted, setIsMounted] = useState(false);
const [renderCnt, setRenderCnt] = useState(0);
const [containerRef, containerRect, containerNode] = useRect();
Expand All @@ -35,8 +36,7 @@ const Supernova = ({ sn, snOptions: options, snPlugins: plugins, layout, appLayo
component.mounted(snNode);
setIsMounted(true);
return () => {
clearTimeout(renderDebouncer.current);
renderDebouncer.current = null;
renderDebouncer.stop();
component.willUnmount();
};
}, [snNode, component]);
Expand All @@ -51,13 +51,7 @@ const Supernova = ({ sn, snOptions: options, snPlugins: plugins, layout, appLayo
return;
}

if (renderDebouncer.current) {
// rendering already scheduled
return;
}

renderDebouncer.current = setTimeout(() => {
// temporarily map constraints to permissions
renderDebouncer.schedule(() => {
const permissions = [];
if (!constraints.passive) {
permissions.push('passive');
Expand All @@ -71,7 +65,7 @@ const Supernova = ({ sn, snOptions: options, snPlugins: plugins, layout, appLayo
if (halo.app && halo.app.session) {
permissions.push('fetch');
}
Promise.resolve(
return Promise.resolve(
component.render({
layout,
options,
Expand All @@ -94,13 +88,12 @@ const Supernova = ({ sn, snOptions: options, snPlugins: plugins, layout, appLayo
},
})
).then(() => {
renderDebouncer.current = null;
if (renderCnt === 0 && typeof options.onInitialRender === 'function') {
options.onInitialRender.call(null);
}
setRenderCnt(renderCnt + 1);
});
}, 10);
});
}, [
containerRect,
options,
Expand Down
79 changes: 79 additions & 0 deletions apis/nucleus/src/utils/__tests__/render-debouncer.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import RenderDebouncer from '../render-debouncer';

function waitForPromise() {
return new Promise((resolve) => {
resolve();
});
}

describe('RenderDebouncer', () => {
let sandbox;
beforeEach(() => {
sandbox = sinon.createSandbox();
sandbox.useFakeTimers();
});
afterEach(() => {
sandbox.restore();
});

// tick the time in 10 ms steps and run resolved promises in between
// workaround for using an old sinon.js that do not support clock.tickAsync
async function tick(time) {
for (let i = 0; i < time / 10; ++i) {
sandbox.clock.tick(Math.min(time - i * 10, 10));
// eslint-disable-next-line no-await-in-loop
await waitForPromise();
}
}

it('should call scheduled function', async () => {
const fn = sinon.stub().resolves();
const debouncer = new RenderDebouncer();
debouncer.schedule(fn);
sandbox.clock.tick(50);
await waitForPromise();
expect(fn).to.be.called;
});

it('should only call second scheduled function if two are scheduled soon after each other', async () => {
const fn1 = sinon.stub().resolves();
const fn2 = sinon.stub().resolves();
const debouncer = new RenderDebouncer();
debouncer.schedule(fn1);
await tick(1);
debouncer.schedule(fn2);
await tick(50);
expect(fn1).to.not.be.called;
expect(fn2).to.be.called;
});

it('should both scheduled function if two are scheduled with some time between', async () => {
const fn1 = sinon.stub().resolves();
const fn2 = sinon.stub().resolves();
const debouncer = new RenderDebouncer();
debouncer.schedule(fn1);
await tick(50);
debouncer.schedule(fn2);
await tick(50);
expect(fn1).to.be.called;
expect(fn2).to.be.called;
});

it('should both scheduled function if two are scheduled with some time between and the first take a long time to run', async () => {
let resolve;
const promise = new Promise((r) => {
resolve = r;
});
const fn1 = sinon.stub().callsFake(() => promise);
const fn2 = sinon.stub().resolves();
const debouncer = new RenderDebouncer();
debouncer.schedule(fn1);
await tick(50);
debouncer.schedule(fn2);
await tick(50);
resolve();
await tick(50);
expect(fn1).to.be.called;
expect(fn2).to.be.called;
});
});
46 changes: 46 additions & 0 deletions apis/nucleus/src/utils/render-debouncer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
export default class RenderDebouncer {
constructor() {
this.timer = null;
this.next = null;
this.running = false;
}

start() {
if (this.running) {
return;
}
this.running = true;
this.scheduleNext();
}

scheduleNext() {
this.timer = setTimeout(() => {
this.doNext();
}, 10);
}

async doNext() {
const fn = this.next;
this.next = null;
if (fn) {
await fn();
this.scheduleNext();
} else {
this.stop();
}
}

schedule(fn) {
this.next = fn;
this.start();
}

stop() {
if (!this.running) {
return;
}
clearTimeout(this.timer);
this.timer = null;
this.running = false;
}
}

0 comments on commit 4811d14

Please sign in to comment.