From b004c20f0ca2ea3b51cb3a3c77d3893f67396819 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Mon, 6 Feb 2017 00:58:16 +0100 Subject: [PATCH 1/5] Make m.redraw() strictly asynchronous --- api/mount.js | 2 +- api/redraw.js | 17 +++-- api/router.js | 10 ++- api/tests/test-mount.js | 83 +++++++++++------------- api/tests/test-redraw.js | 68 +++++++++++++------- api/tests/test-router.js | 91 +++++++++++++++++++-------- docs/redraw.md | 2 +- docs/route.md | 2 +- test-utils/tests/test-throttleMock.js | 89 ++++++++++++++++++++++++++ test-utils/throttleMock.js | 24 +++++++ tests/test-api.js | 4 +- 11 files changed, 281 insertions(+), 111 deletions(-) create mode 100644 test-utils/tests/test-throttleMock.js create mode 100644 test-utils/throttleMock.js diff --git a/api/mount.js b/api/mount.js index 2178505a..7203bf7c 100644 --- a/api/mount.js +++ b/api/mount.js @@ -16,6 +16,6 @@ module.exports = function(redrawService) { redrawService.render(root, Vnode(component)) } redrawService.subscribe(root, run) - redrawService.redraw() + run() } } diff --git a/api/redraw.js b/api/redraw.js index 1b22271b..07937445 100644 --- a/api/redraw.js +++ b/api/redraw.js @@ -4,26 +4,23 @@ var coreRenderer = require("../render/render") function throttle(callback) { //60fps translates to 16.6ms, round it down since setTimeout requires int - var time = 16 + var delay = 16 var last = 0, pending = null var timeout = typeof requestAnimationFrame === "function" ? requestAnimationFrame : setTimeout return function() { - var now = Date.now() - if (last === 0 || now - last >= time) { - last = now - callback() - } - else if (pending === null) { + var elapsed = Date.now() - last + if (pending === null) { pending = timeout(function() { pending = null callback() last = Date.now() - }, time - (now - last)) + }, delay - elapsed) } } } -module.exports = function($window) { +module.exports = function($window, throttleMock) { + var _throttle = throttleMock || throttle var renderService = coreRenderer($window) renderService.setEventCallback(function(e) { if (e.redraw !== false) redraw() @@ -32,7 +29,7 @@ module.exports = function($window) { var callbacks = [] function subscribe(key, callback) { unsubscribe(key) - callbacks.push(key, throttle(callback)) + callbacks.push(key, _throttle(callback)) } function unsubscribe(key) { var index = callbacks.indexOf(key) diff --git a/api/router.js b/api/router.js index c182008f..dbdd3c89 100644 --- a/api/router.js +++ b/api/router.js @@ -11,9 +11,14 @@ module.exports = function($window, redrawService) { var render, component, attrs, currentPath, lastUpdate var route = function(root, defaultRoute, routes) { if (root == null) throw new Error("Ensure the DOM element that was passed to `m.route` is not undefined") - var run = function() { + function run() { if (render != null) redrawService.render(root, render(Vnode(component, attrs.key, attrs))) } + var redraw = function() { + run() + redraw = redrawService.redraw + } + redrawService.subscribe(root, run) var bail = function(path) { if (path !== defaultRoute) routeService.setPath(defaultRoute, null, {replace: true}) else throw new Error("Could not resolve default route " + defaultRoute) @@ -24,7 +29,7 @@ module.exports = function($window, redrawService) { component = comp != null && (typeof comp.view === "function" || typeof comp === "function")? comp : "div" attrs = params, currentPath = path, lastUpdate = null render = (routeResolver.render || identity).bind(routeResolver) - run() + redraw() } if (payload.view || typeof payload === "function") update({}, payload) else { @@ -36,7 +41,6 @@ module.exports = function($window, redrawService) { else update(payload, "div") } }, bail) - redrawService.subscribe(root, run) } route.set = function(path, data, options) { if (lastUpdate != null) options = {replace: true} diff --git a/api/tests/test-mount.js b/api/tests/test-mount.js index db2bee04..e115711b 100644 --- a/api/tests/test-mount.js +++ b/api/tests/test-mount.js @@ -3,6 +3,7 @@ var o = require("../../ospec/ospec") var components = require("../../test-utils/components") var domMock = require("../../test-utils/domMock") +var throttleMocker = require("../../test-utils/throttleMock") var m = require("../../render/hyperscript") var coreRenderer = require("../../render/render") @@ -11,18 +12,22 @@ var apiMounter = require("../../api/mount") o.spec("mount", function() { var FRAME_BUDGET = Math.floor(1000 / 60) - var $window, root, redrawService, mount, render + var $window, root, redrawService, mount, render, throttleMock o.beforeEach(function() { $window = domMock() + throttleMock = throttleMocker() root = $window.document.body - - redrawService = apiRedraw($window) + redrawService = apiRedraw($window, throttleMock.throttle) mount = apiMounter(redrawService) render = coreRenderer($window).render }) + o.afterEach(function() { + o(throttleMock.queueLength()).equals(0) + }) + o("throws on invalid component", function() { var threw = false try { @@ -69,7 +74,7 @@ o.spec("mount", function() { o(root.childNodes.length).equals(0) }) - o("redraws on events", function(done) { + o("redraws on events", function() { var onupdate = o.spy() var oninit = o.spy() var onclick = o.spy() @@ -97,17 +102,12 @@ o.spec("mount", function() { o(onclick.args[0].type).equals("click") o(onclick.args[0].target).equals(root.firstChild) - // Wrapped to give time for the rate-limited redraw to fire - setTimeout(function() { - o(onupdate.callCount).equals(1) + throttleMock.fire() - done() - }, FRAME_BUDGET) + o(onupdate.callCount).equals(1) }) - o("redraws several mount points on events", function(done, timeout) { - timeout(60) - + o("redraws several mount points on events", function() { var onupdate0 = o.spy() var oninit0 = o.spy() var onclick0 = o.spy() @@ -154,26 +154,26 @@ o.spec("mount", function() { o(onclick0.callCount).equals(1) o(onclick0.this).equals(root.childNodes[0].firstChild) - setTimeout(function() { - o(onupdate0.callCount).equals(1) - o(onupdate1.callCount).equals(1) + throttleMock.fire() - root.childNodes[1].firstChild.dispatchEvent(e) - o(onclick1.callCount).equals(1) - o(onclick1.this).equals(root.childNodes[1].firstChild) + o(onupdate0.callCount).equals(1) + o(onupdate1.callCount).equals(1) - setTimeout(function() { - o(onupdate0.callCount).equals(2) - o(onupdate1.callCount).equals(2) + root.childNodes[1].firstChild.dispatchEvent(e) - done() - }, FRAME_BUDGET) - }, FRAME_BUDGET) + o(onclick1.callCount).equals(1) + o(onclick1.this).equals(root.childNodes[1].firstChild) + throttleMock.fire() + + o(onupdate0.callCount).equals(2) + o(onupdate1.callCount).equals(2) }) - o("event handlers can skip redraw", function(done) { - var onupdate = o.spy() + o("event handlers can skip redraw", function() { + var onupdate = o.spy(function(){ + throw new Error("This shouldn't have been called") + }) var oninit = o.spy() var e = $window.document.createEvent("MouseEvents") @@ -195,15 +195,12 @@ o.spec("mount", function() { o(oninit.callCount).equals(1) - // Wrapped to ensure no redraw fired - setTimeout(function() { - o(onupdate.callCount).equals(0) + throttleMock.fire() - done() - }, FRAME_BUDGET) + o(onupdate.callCount).equals(0) }) - o("redraws when the render function is run", function(done) { + o("redraws when the render function is run", function() { var onupdate = o.spy() var oninit = o.spy() @@ -221,17 +218,12 @@ o.spec("mount", function() { redrawService.redraw() - // Wrapped to give time for the rate-limited redraw to fire - setTimeout(function() { - o(onupdate.callCount).equals(1) + throttleMock.fire() - done() - }, FRAME_BUDGET) + o(onupdate.callCount).equals(1) }) - o("throttles", function(done, timeout) { - timeout(200) - + o("throttles", function() { var i = 0 mount(root, createComponent({view: function() {i++}})) var before = i @@ -243,12 +235,11 @@ o.spec("mount", function() { var after = i - setTimeout(function(){ - o(before).equals(1) // mounts synchronously - o(after).equals(1) // throttles rest - o(i).equals(2) - done() - },40) + throttleMock.fire() + + o(before).equals(1) // mounts synchronously + o(after).equals(1) // throttles rest + o(i).equals(2) }) }) }) diff --git a/api/tests/test-redraw.js b/api/tests/test-redraw.js index f13c2d3f..80a768dc 100644 --- a/api/tests/test-redraw.js +++ b/api/tests/test-redraw.js @@ -2,6 +2,7 @@ var o = require("../../ospec/ospec") var domMock = require("../../test-utils/domMock") +var throttleMocker = require("../../test-utils/throttleMock") var apiRedraw = require("../../api/redraw") o.spec("redrawService", function() { @@ -17,25 +18,39 @@ o.spec("redrawService", function() { redrawService.redraw() }) + o("honours throttleMock", function() { + var throttleMock = throttleMocker() + redrawService = apiRedraw(domMock(), throttleMock.throttle) + var spy = o.spy() + + redrawService.subscribe(root, spy) + + o(spy.callCount).equals(0) + + redrawService.redraw() + + o(spy.callCount).equals(0) + + throttleMock.fire() + + o(spy.callCount).equals(1) + }) + o("should run a single renderer entry", function(done) { var spy = o.spy() redrawService.subscribe(root, spy) o(spy.callCount).equals(0) - - redrawService.redraw() - - o(spy.callCount).equals(1) redrawService.redraw() redrawService.redraw() redrawService.redraw() - o(spy.callCount).equals(1) + o(spy.callCount).equals(0) setTimeout(function() { - o(spy.callCount).equals(2) - + o(spy.callCount).equals(1) + done() }, 20) }) @@ -54,27 +69,29 @@ o.spec("redrawService", function() { redrawService.redraw() - o(spy1.callCount).equals(1) - o(spy2.callCount).equals(1) - o(spy3.callCount).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + o(spy3.callCount).equals(0) redrawService.redraw() - o(spy1.callCount).equals(1) - o(spy2.callCount).equals(1) - o(spy3.callCount).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + o(spy3.callCount).equals(0) setTimeout(function() { - o(spy1.callCount).equals(2) - o(spy2.callCount).equals(2) - o(spy3.callCount).equals(2) - + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + o(spy3.callCount).equals(1) + done() }, 20) }) - o("should stop running after unsubscribe", function() { - var spy = o.spy() + o("should stop running after unsubscribe", function(done) { + var spy = o.spy(function() { + throw new Error("This shouldn't have been called") + }) redrawService.subscribe(root, spy) redrawService.unsubscribe(root, spy) @@ -82,9 +99,14 @@ o.spec("redrawService", function() { redrawService.redraw() o(spy.callCount).equals(0) + setTimeout(function() { + o(spy.callCount).equals(0) + + done() + }, 20) }) - o("does nothing on invalid unsubscribe", function() { + o("does nothing on invalid unsubscribe", function(done) { var spy = o.spy() redrawService.subscribe(root, spy) @@ -92,6 +114,10 @@ o.spec("redrawService", function() { redrawService.redraw() - o(spy.callCount).equals(1) + setTimeout(function() { + o(spy.callCount).equals(1) + + done() + }, 20) }) }) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 9406de70..bc3e7023 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -3,6 +3,7 @@ var o = require("../../ospec/ospec") var callAsync = require("../../test-utils/callAsync") var browserMock = require("../../test-utils/browserMock") +var throttleMocker = require("../../test-utils/throttleMock") var m = require("../../render/hyperscript") var callAsync = require("../../test-utils/callAsync") @@ -15,18 +16,23 @@ o.spec("route", function() { void ["#", "?", "", "#!", "?!", "/foo"].forEach(function(prefix) { o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() { var FRAME_BUDGET = Math.floor(1000 / 60) - var $window, root, redrawService, route + var $window, root, redrawService, route, throttleMock o.beforeEach(function() { $window = browserMock(env) + throttleMock = throttleMocker() root = $window.document.body - redrawService = apiRedraw($window) + redrawService = apiRedraw($window, throttleMock.throttle) route = apiRouter($window, redrawService) route.prefix(prefix) }) + o.afterEach(function() { + o(throttleMock.queueLength()).equals(0) + }) + o("throws on invalid `root` DOM node", function() { var threw = false try { @@ -50,7 +56,7 @@ o.spec("route", function() { o(root.firstChild.nodeName).equals("DIV") }) - o("routed mount points can redraw synchronously (POJO component)", function() { + o("routed mount points only redraw asynchronously (POJO component)", function() { var view = o.spy() $window.location.href = prefix + "/" @@ -60,11 +66,14 @@ o.spec("route", function() { redrawService.redraw() - o(view.callCount).equals(2) + o(view.callCount).equals(1) + throttleMock.fire() + + o(view.callCount).equals(2) }) - o("routed mount points can redraw synchronously (constructible component)", function() { + o("routed mount points only redraw asynchronously (constructible component)", function() { var view = o.spy() var Cmp = function(){} @@ -77,11 +86,14 @@ o.spec("route", function() { redrawService.redraw() - o(view.callCount).equals(2) + o(view.callCount).equals(1) + throttleMock.fire() + + o(view.callCount).equals(2) }) - o("routed mount points can redraw synchronously (closure component)", function() { + o("routed mount points only redraw asynchronously (closure component)", function() { var view = o.spy() function Cmp() {return {view: view}} @@ -93,8 +105,11 @@ o.spec("route", function() { redrawService.redraw() - o(view.callCount).equals(2) + o(view.callCount).equals(1) + throttleMock.fire() + + o(view.callCount).equals(2) }) o("default route doesn't break back button", function(done) { @@ -160,11 +175,12 @@ o.spec("route", function() { o(oninit.callCount).equals(1) redrawService.redraw() + throttleMock.fire() o(onupdate.callCount).equals(1) }) - o("redraws on events", function(done) { + o("redraws on events", function() { var onupdate = o.spy() var oninit = o.spy() var onclick = o.spy() @@ -194,12 +210,9 @@ o.spec("route", function() { o(onclick.args[0].type).equals("click") o(onclick.args[0].target).equals(root.firstChild) - // Wrapped to give time for the rate-limited redraw to fire - callAsync(function() { - o(onupdate.callCount).equals(1) - done() - }) + throttleMock.fire() + o(onupdate.callCount).equals(1) }) o("event handlers can skip redraw", function(done) { @@ -500,7 +513,10 @@ o.spec("route", function() { o(oninit.callCount).equals(1) route.set("/def") callAsync(function() { + throttleMock.fire() + o(oninit.callCount).equals(2) + done() }) }) @@ -536,23 +552,28 @@ o.spec("route", function() { route(root, "/a", { "/a" : { render: function() { - return m("div") + return m("div", m('p')) }, }, "/b" : { render: function() { - return m("div") + return m("div", m('a')) }, }, }) var dom = root.firstChild + var child = dom.firstChild + o(root.firstChild.nodeName).equals("DIV") route.set("/b") callAsync(function() { + throttleMock.fire() + o(root.firstChild).equals(dom) + o(root.firstChild.firstChild).notEquals(child) done() }) @@ -586,6 +607,7 @@ o.spec("route", function() { o(renderCount).equals(1) redrawService.redraw() + throttleMock.fire() o(matchCount).equals(1) o(renderCount).equals(2) @@ -621,6 +643,7 @@ o.spec("route", function() { o(renderCount).equals(1) redrawService.redraw() + throttleMock.fire() o(matchCount).equals(1) o(renderCount).equals(2) @@ -815,10 +838,14 @@ o.spec("route", function() { }) callAsync(function() { - route.set("/b") + throttleMock.fire() + + route.set('/b') callAsync(function() { callAsync(function() { callAsync(function() { + throttleMock.fire() + o(render.callCount).equals(0) o(component.view.callCount).equals(2) @@ -939,6 +966,7 @@ o.spec("route", function() { o(onmatch.callCount).equals(1) redrawService.redraw() + throttleMock.fire() o(view.callCount).equals(2) o(onmatch.callCount).equals(1) @@ -1017,6 +1045,8 @@ o.spec("route", function() { }) callAsync(function() { + throttleMock.fire() + o(onmatch.callCount).equals(1) o(render.callCount).equals(1) @@ -1024,6 +1054,8 @@ o.spec("route", function() { callAsync(function() { callAsync(function() { + throttleMock.fire() + o(onmatch.callCount).equals(2) o(render.callCount).equals(2) @@ -1074,9 +1106,15 @@ o.spec("route", function() { route.set("/b") callAsync(function() { - route.set("/a") + throttleMock.fire() + + o(root.firstChild.nodeName).equals("B") + + route.set('/a') callAsync(function() { + throttleMock.fire() + o(root.firstChild.nodeName).equals("A") done() @@ -1141,7 +1179,9 @@ o.spec("route", function() { route.set("/b") + // setting the route is asynchronous callAsync(function() { + throttleMock.fire() o(spy.callCount).equals(1) done() @@ -1181,9 +1221,7 @@ o.spec("route", function() { }) }) - o("throttles", function(done, timeout) { - timeout(200) - + o("throttles", function() { var i = 0 $window.location.href = prefix + "/" route(root, "/", { @@ -1197,12 +1235,11 @@ o.spec("route", function() { redrawService.redraw() var after = i - setTimeout(function() { - o(before).equals(1) // routes synchronously - o(after).equals(2) // redraws synchronously - o(i).equals(3) // throttles rest - done() - }, FRAME_BUDGET * 2) + throttleMock.fire() + + o(before).equals(1) // routes synchronously + o(after).equals(1) // redraws asynchronously + o(i).equals(2) }) o("m.route.param is available outside of route handlers", function(done) { diff --git a/docs/redraw.md b/docs/redraw.md index 85b397dc..7bb25810 100644 --- a/docs/redraw.md +++ b/docs/redraw.md @@ -14,7 +14,7 @@ You DON'T need to call it if data is modified within the execution context of an You DO need to call it in `setTimeout`/`setInterval`/`requestAnimationFrame` callbacks, or callbacks from 3rd party libraries. -Typically, `m.redraw` triggers an asynchronous redraws, but it may trigger synchronously if Mithril detects it's possible to improve performance by doing so (i.e. if no redraw was requested within the last animation frame). You should write code assuming that it always redraws asynchronously. +`m.redraw` always triggers an asynchronous redraws. --- diff --git a/docs/route.md b/docs/route.md index 6f5c571b..16a08eec 100644 --- a/docs/route.md +++ b/docs/route.md @@ -65,7 +65,7 @@ Argument | Type | Required | D ##### m.route.set -Redirects to a matching route, or to the default route if no matching routes can be found. +Redirects to a matching route, or to the default route if no matching routes can be found. Triggers an asynchronous redraw off all mount points. `m.route.set(path, data, options)` diff --git a/test-utils/tests/test-throttleMock.js b/test-utils/tests/test-throttleMock.js new file mode 100644 index 00000000..8126af05 --- /dev/null +++ b/test-utils/tests/test-throttleMock.js @@ -0,0 +1,89 @@ +var o = require("../../ospec/ospec") +var throttleMocker = require("../../test-utils/throttleMock") + +o.spec("throttleMock", function() { + o("works with one callback", function() { + var throttleMock = throttleMocker() + var spy = o.spy() + + o(throttleMock.queueLength()).equals(0) + + var throttled = throttleMock.throttle(spy) + + o(throttleMock.queueLength()).equals(0) + o(spy.callCount).equals(0) + + throttled() + + o(throttleMock.queueLength()).equals(1) + o(spy.callCount).equals(0) + + throttled() + + o(throttleMock.queueLength()).equals(1) + o(spy.callCount).equals(0) + + throttleMock.fire() + + o(throttleMock.queueLength()).equals(0) + o(spy.callCount).equals(1) + + throttleMock.fire() + + o(spy.callCount).equals(1) + }) + o("works with two callbacks", function() { + var throttleMock = throttleMocker() + var spy1 = o.spy() + var spy2 = o.spy() + + o(throttleMock.queueLength()).equals(0) + + var throttled1 = throttleMock.throttle(spy1) + + o(throttleMock.queueLength()).equals(0) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + + throttled1() + + o(throttleMock.queueLength()).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + + throttled1() + + o(throttleMock.queueLength()).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + + var throttled2 = throttleMock.throttle(spy2) + + o(throttleMock.queueLength()).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + + throttled2() + + o(throttleMock.queueLength()).equals(2) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + + throttled2() + + o(throttleMock.queueLength()).equals(2) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + + throttleMock.fire() + + o(throttleMock.queueLength()).equals(0) + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + + throttleMock.fire() + + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + }) +}) diff --git a/test-utils/throttleMock.js b/test-utils/throttleMock.js new file mode 100644 index 00000000..46076b28 --- /dev/null +++ b/test-utils/throttleMock.js @@ -0,0 +1,24 @@ +module.exports = function() { + var queue = [] + return { + throttle: function(fn) { + var pending = false + return function() { + if (!pending) { + queue.push(function(){ + pending = false + fn() + }) + pending = true + } + } + }, + fire: function() { + queue.forEach(function(fn) {fn()}) + queue.length = 0 + }, + queueLength: function(){ + return queue.length + } + } +} diff --git a/tests/test-api.js b/tests/test-api.js index 3240141c..d0c2cf81 100644 --- a/tests/test-api.js +++ b/tests/test-api.js @@ -163,8 +163,10 @@ o.spec("api", function() { var count = 0 var root = window.document.createElement("div") m.mount(root, createComponent({view: function() {count++}})) + o(count).equals(1) + m.redraw() + o(count).equals(1) setTimeout(function() { - m.redraw() o(count).equals(2) From ccb3d61675da138be61bab83f44bc4c26cdc8ecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Yves=20G=C3=A9rardy?= Date: Tue, 13 Jun 2017 15:35:05 +0200 Subject: [PATCH 2/5] Make redraw monolithic, add m.redraw.sync --- api/redraw.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/api/redraw.js b/api/redraw.js index 07937445..33472635 100644 --- a/api/redraw.js +++ b/api/redraw.js @@ -19,26 +19,32 @@ function throttle(callback) { } } + module.exports = function($window, throttleMock) { - var _throttle = throttleMock || throttle var renderService = coreRenderer($window) renderService.setEventCallback(function(e) { if (e.redraw !== false) redraw() }) var callbacks = [] + var rendering = false + function subscribe(key, callback) { unsubscribe(key) - callbacks.push(key, _throttle(callback)) + callbacks.push(key, callback) } function unsubscribe(key) { var index = callbacks.indexOf(key) if (index > -1) callbacks.splice(index, 2) } - function redraw() { - for (var i = 1; i < callbacks.length; i += 2) { - callbacks[i]() - } + function sync() { + if (rendering) throw new Error("Nested m.redraw.sync() call") + rendering = true + for (var i = 1; i < callbacks.length; i+=2) try {callbacks[i]()} catch (e) {/*noop*/} + rendering = false } + + var redraw = (throttleMock || throttle)(sync) + redraw.sync = sync return {subscribe: subscribe, unsubscribe: unsubscribe, redraw: redraw, render: renderService.render} } From 7de012433936ce5a29db5cad4bbb81491eea554b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Yves=20G=C3=A9rardy?= Date: Tue, 13 Jun 2017 16:29:02 +0200 Subject: [PATCH 3/5] Tests for m.redraw.sync() --- api/tests/test-mount.js | 32 ++++++++++++++++++++++++++- api/tests/test-redraw.js | 48 ++++++++++++++++++++++++++++++++++++++++ tests/test-api.js | 8 +++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/api/tests/test-mount.js b/api/tests/test-mount.js index e115711b..80ade907 100644 --- a/api/tests/test-mount.js +++ b/api/tests/test-mount.js @@ -52,7 +52,7 @@ o.spec("mount", function() { o(threw).equals(true) }) - o("renders into `root`", function() { + o("renders into `root` synchronoulsy", function() { mount(root, createComponent({ view : function() { return m("div") @@ -74,6 +74,36 @@ o.spec("mount", function() { o(root.childNodes.length).equals(0) }) + o("Mounting a second root doesn't cause the first one to redraw", function() { + var view = o.spy(function() { + return m("div") + }) + + render(root, [ + m("#child0"), + m("#child1") + ]) + + mount(root.childNodes[0], createComponent({ + view : view + })) + + o(root.firstChild.nodeName).equals("DIV") + o(view.callCount).equals(1) + + mount(root.childNodes[1], createComponent({ + view : function() { + return m("div") + } + })) + + o(view.callCount).equals(1) + + throttleMock.fire() + + o(view.callCount).equals(1) + }) + o("redraws on events", function() { var onupdate = o.spy() var oninit = o.spy() diff --git a/api/tests/test-redraw.js b/api/tests/test-redraw.js index 80a768dc..68b1a911 100644 --- a/api/tests/test-redraw.js +++ b/api/tests/test-redraw.js @@ -106,6 +106,25 @@ o.spec("redrawService", function() { }, 20) }) + o("should stop running after unsubscribe, even if it occurs after redraw is requested", function(done) { + var spy = o.spy(function() { + throw new Error("This shouldn't have been called") + }) + + redrawService.subscribe(root, spy) + + redrawService.redraw() + + redrawService.unsubscribe(root, spy) + + o(spy.callCount).equals(0) + setTimeout(function() { + o(spy.callCount).equals(0) + + done() + }, 20) + }) + o("does nothing on invalid unsubscribe", function(done) { var spy = o.spy() @@ -120,4 +139,33 @@ o.spec("redrawService", function() { done() }, 20) }) + + o("redraw.sync() redraws all roots synchronously", function() { + var el1 = $document.createElement("div") + var el2 = $document.createElement("div") + var el3 = $document.createElement("div") + var spy1 = o.spy() + var spy2 = o.spy() + var spy3 = o.spy() + + redrawService.subscribe(el1, spy1) + redrawService.subscribe(el2, spy2) + redrawService.subscribe(el3, spy3) + + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) + o(spy3.callCount).equals(0) + + redrawService.redraw.sync() + + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + o(spy3.callCount).equals(1) + + redrawService.redraw.sync() + + o(spy1.callCount).equals(2) + o(spy2.callCount).equals(2) + o(spy3.callCount).equals(2) + }) }) diff --git a/tests/test-api.js b/tests/test-api.js index d0c2cf81..49938d82 100644 --- a/tests/test-api.js +++ b/tests/test-api.js @@ -173,6 +173,14 @@ o.spec("api", function() { done() }, FRAME_BUDGET) }) + o("sync", function() { + var root = window.document.createElement("div") + var view = o.spy() + m.mount(root, createComponent({view: view})) + o(view.callCount).equals(1) + m.redraw.sync() + o(view.callCount).equals(2) + }) }) }) }) From 47d59ea68a266f7eec296e855cedead7542ff122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Yves=20G=C3=A9rardy?= Date: Tue, 13 Jun 2017 23:30:12 +0200 Subject: [PATCH 4/5] [test-utils] Make throttleMock more reliable --- test-utils/throttleMock.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test-utils/throttleMock.js b/test-utils/throttleMock.js index 46076b28..06e74f99 100644 --- a/test-utils/throttleMock.js +++ b/test-utils/throttleMock.js @@ -14,8 +14,9 @@ module.exports = function() { } }, fire: function() { - queue.forEach(function(fn) {fn()}) - queue.length = 0 + var tasks = queue + queue = [] + tasks.forEach(function(fn) {fn()}) }, queueLength: function(){ return queue.length From 0e0ed7c45d658820f8317e8c1041f9e2d87d5ea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Yves=20G=C3=A9rardy?= Date: Wed, 14 Jun 2017 00:15:08 +0200 Subject: [PATCH 5/5] Lint --- api/tests/test-mount.js | 1 - api/tests/test-redraw.js | 2 +- api/tests/test-router.js | 9 +- test-utils/tests/test-throttleMock.js | 114 +++++++++++++------------- test-utils/throttleMock.js | 48 +++++------ 5 files changed, 88 insertions(+), 86 deletions(-) diff --git a/api/tests/test-mount.js b/api/tests/test-mount.js index 80ade907..6bc69ce9 100644 --- a/api/tests/test-mount.js +++ b/api/tests/test-mount.js @@ -11,7 +11,6 @@ var apiRedraw = require("../../api/redraw") var apiMounter = require("../../api/mount") o.spec("mount", function() { - var FRAME_BUDGET = Math.floor(1000 / 60) var $window, root, redrawService, mount, render, throttleMock o.beforeEach(function() { diff --git a/api/tests/test-redraw.js b/api/tests/test-redraw.js index 68b1a911..65831bb0 100644 --- a/api/tests/test-redraw.js +++ b/api/tests/test-redraw.js @@ -166,6 +166,6 @@ o.spec("redrawService", function() { o(spy1.callCount).equals(2) o(spy2.callCount).equals(2) - o(spy3.callCount).equals(2) + o(spy3.callCount).equals(2) }) }) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index bc3e7023..ad009667 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -15,7 +15,6 @@ o.spec("route", function() { void [{protocol: "http:", hostname: "localhost"}, {protocol: "file:", hostname: "/"}].forEach(function(env) { void ["#", "?", "", "#!", "?!", "/foo"].forEach(function(prefix) { o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() { - var FRAME_BUDGET = Math.floor(1000 / 60) var $window, root, redrawService, route, throttleMock o.beforeEach(function() { @@ -552,12 +551,12 @@ o.spec("route", function() { route(root, "/a", { "/a" : { render: function() { - return m("div", m('p')) + return m("div", m("p")) }, }, "/b" : { render: function() { - return m("div", m('a')) + return m("div", m("a")) }, }, }) @@ -840,7 +839,7 @@ o.spec("route", function() { callAsync(function() { throttleMock.fire() - route.set('/b') + route.set("/b") callAsync(function() { callAsync(function() { callAsync(function() { @@ -1110,7 +1109,7 @@ o.spec("route", function() { o(root.firstChild.nodeName).equals("B") - route.set('/a') + route.set("/a") callAsync(function() { throttleMock.fire() diff --git a/test-utils/tests/test-throttleMock.js b/test-utils/tests/test-throttleMock.js index 8126af05..69920623 100644 --- a/test-utils/tests/test-throttleMock.js +++ b/test-utils/tests/test-throttleMock.js @@ -1,89 +1,91 @@ +"use strict" + var o = require("../../ospec/ospec") var throttleMocker = require("../../test-utils/throttleMock") o.spec("throttleMock", function() { - o("works with one callback", function() { - var throttleMock = throttleMocker() - var spy = o.spy() + o("works with one callback", function() { + var throttleMock = throttleMocker() + var spy = o.spy() - o(throttleMock.queueLength()).equals(0) + o(throttleMock.queueLength()).equals(0) - var throttled = throttleMock.throttle(spy) + var throttled = throttleMock.throttle(spy) - o(throttleMock.queueLength()).equals(0) - o(spy.callCount).equals(0) + o(throttleMock.queueLength()).equals(0) + o(spy.callCount).equals(0) - throttled() + throttled() - o(throttleMock.queueLength()).equals(1) - o(spy.callCount).equals(0) + o(throttleMock.queueLength()).equals(1) + o(spy.callCount).equals(0) - throttled() + throttled() - o(throttleMock.queueLength()).equals(1) - o(spy.callCount).equals(0) + o(throttleMock.queueLength()).equals(1) + o(spy.callCount).equals(0) - throttleMock.fire() + throttleMock.fire() - o(throttleMock.queueLength()).equals(0) - o(spy.callCount).equals(1) + o(throttleMock.queueLength()).equals(0) + o(spy.callCount).equals(1) - throttleMock.fire() + throttleMock.fire() - o(spy.callCount).equals(1) - }) - o("works with two callbacks", function() { - var throttleMock = throttleMocker() - var spy1 = o.spy() - var spy2 = o.spy() + o(spy.callCount).equals(1) + }) + o("works with two callbacks", function() { + var throttleMock = throttleMocker() + var spy1 = o.spy() + var spy2 = o.spy() - o(throttleMock.queueLength()).equals(0) + o(throttleMock.queueLength()).equals(0) - var throttled1 = throttleMock.throttle(spy1) + var throttled1 = throttleMock.throttle(spy1) - o(throttleMock.queueLength()).equals(0) - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) + o(throttleMock.queueLength()).equals(0) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) - throttled1() + throttled1() - o(throttleMock.queueLength()).equals(1) - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) + o(throttleMock.queueLength()).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) - throttled1() + throttled1() - o(throttleMock.queueLength()).equals(1) - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) + o(throttleMock.queueLength()).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) - var throttled2 = throttleMock.throttle(spy2) + var throttled2 = throttleMock.throttle(spy2) - o(throttleMock.queueLength()).equals(1) - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) + o(throttleMock.queueLength()).equals(1) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) - throttled2() + throttled2() - o(throttleMock.queueLength()).equals(2) - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) + o(throttleMock.queueLength()).equals(2) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) - throttled2() + throttled2() - o(throttleMock.queueLength()).equals(2) - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) + o(throttleMock.queueLength()).equals(2) + o(spy1.callCount).equals(0) + o(spy2.callCount).equals(0) - throttleMock.fire() + throttleMock.fire() - o(throttleMock.queueLength()).equals(0) - o(spy1.callCount).equals(1) - o(spy2.callCount).equals(1) + o(throttleMock.queueLength()).equals(0) + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) - throttleMock.fire() + throttleMock.fire() - o(spy1.callCount).equals(1) - o(spy2.callCount).equals(1) - }) + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + }) }) diff --git a/test-utils/throttleMock.js b/test-utils/throttleMock.js index 06e74f99..6cdb5710 100644 --- a/test-utils/throttleMock.js +++ b/test-utils/throttleMock.js @@ -1,25 +1,27 @@ +"use strict" + module.exports = function() { - var queue = [] - return { - throttle: function(fn) { - var pending = false - return function() { - if (!pending) { - queue.push(function(){ - pending = false - fn() - }) - pending = true - } - } - }, - fire: function() { - var tasks = queue - queue = [] - tasks.forEach(function(fn) {fn()}) - }, - queueLength: function(){ - return queue.length - } - } + var queue = [] + return { + throttle: function(fn) { + var pending = false + return function() { + if (!pending) { + queue.push(function(){ + pending = false + fn() + }) + pending = true + } + } + }, + fire: function() { + var tasks = queue + queue = [] + tasks.forEach(function(fn) {fn()}) + }, + queueLength: function(){ + return queue.length + } + } }