From 90bcff0fa7dc3683c2e7a97382f4ddf7f3c77d35 Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Fri, 5 Jul 2019 18:52:06 -0400 Subject: [PATCH] Deduplicate `m.route` and `m.redraw` logic (#2453) - Remove appropriate route change subcriptions when a root is removed via `m.mount(root, null)`. - Don't pollute `onpopstate` and friends - use standard event listeners instead. - Simplify and streamline subscriptions, in preparation of adding a `remove` parameter to `m.mount`. - Change the redraw internals to redraw immediately, with ability to cancel via returning a sentinel. - Change `"bleeding-edge"` for `m.version` in `next` to instead just be the latest `m.version`. (If you're using `next`, you should know what you're in for.) - Update tests to be aware of these changes. (Some were failing for subtle reasons.) - Drive-by: remove some uses of `string.charAt(n)` and use `string[n]` instead. --- api/mount.js | 14 +-- api/redraw.js | 30 ++++-- api/router.js | 37 ++++--- api/tests/test-redraw.js | 81 ++++++++------- api/tests/test-router.js | 19 ++++ bundler/bundle.js | 8 +- bundler/tests/test-bundler.js | 8 ++ docs/change-log.md | 4 + docs/route.md | 79 +++++++++++++++ index.js | 2 +- request/tests/test-request.js | 24 +++-- router/router.js | 158 ++++++++++++++++-------------- router/tests/test-defineRoutes.js | 41 ++++---- router/tests/test-getPath.js | 12 ++- router/tests/test-setPath.js | 26 ++--- test-utils/domMock.js | 8 ++ test-utils/pushStateMock.js | 8 ++ tests/test-api.js | 30 +++--- 18 files changed, 397 insertions(+), 192 deletions(-) diff --git a/api/mount.js b/api/mount.js index 7203bf7c..ab8ecc13 100644 --- a/api/mount.js +++ b/api/mount.js @@ -5,17 +5,11 @@ var Vnode = require("../render/vnode") module.exports = function(redrawService) { return function(root, component) { if (component === null) { - redrawService.render(root, []) redrawService.unsubscribe(root) - return + } else if (component.view == null && typeof component !== "function") { + throw new Error("m.mount(element, component) expects a component, not a vnode") + } else { + redrawService.subscribe(root, function() { return Vnode(component) }) } - - if (component.view == null && typeof component !== "function") throw new Error("m.mount(element, component) expects a component, not a vnode") - - var run = function() { - redrawService.render(root, Vnode(component)) - } - redrawService.subscribe(root, run) - run() } } diff --git a/api/redraw.js b/api/redraw.js index e43ffffc..235157f3 100644 --- a/api/redraw.js +++ b/api/redraw.js @@ -14,24 +14,40 @@ function throttle(callback) { } } - module.exports = function($window, throttleMock) { var renderService = coreRenderer($window) - var callbacks = [] + var subscriptions = [] var rendering = false - function subscribe(key, callback) { + function run(sub) { + var vnode = sub.c(sub) + if (vnode !== sub) renderService.render(sub.k, vnode) + } + function subscribe(key, callback, onremove) { + var sub = {k: key, c: callback, r: onremove} unsubscribe(key) - callbacks.push(key, callback) + subscriptions.push(sub) + var vnode = sub.c(sub) + if (vnode !== sub) renderService.render(sub.k, vnode) } function unsubscribe(key) { - var index = callbacks.indexOf(key) - if (index > -1) callbacks.splice(index, 2) + for (var i = 0; i < subscriptions.length; i++) { + var sub = subscriptions[i] + if (sub.k === key) { + subscriptions.splice(i, 1) + renderService.render(sub.k, []) + if (typeof sub.r === "function") sub.r() + break + } + } } 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) {if (typeof console !== "undefined") console.error(e)} + for (var i = 0; i < subscriptions.length; i++) { + try { run(subscriptions[i]) } + catch (e) { if (typeof console !== "undefined") console.error(e) } + } rendering = false } diff --git a/api/router.js b/api/router.js index a053bac2..a388b40c 100644 --- a/api/router.js +++ b/api/router.js @@ -4,32 +4,38 @@ var Vnode = require("../render/vnode") var Promise = require("../promise/promise") var coreRouter = require("../router/router") +var sentinel = {} + module.exports = function($window, redrawService) { var routeService = coreRouter($window) - var identity = function(v) {return v} - var render, component, attrs, currentPath, lastUpdate + var currentResolver = sentinel, 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") - 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 init = false var bail = function(path) { if (path !== defaultRoute) routeService.setPath(defaultRoute, null, {replace: true}) else throw new Error("Could not resolve default route " + defaultRoute) } + function run() { + init = true + if (sentinel !== currentResolver) { + var vnode = Vnode(component, attrs.key, attrs) + if (currentResolver) vnode = currentResolver.render(vnode) + return vnode + } + } routeService.defineRoutes(routes, function(payload, params, path, route) { var update = lastUpdate = function(routeResolver, comp) { if (update !== lastUpdate) return component = comp != null && (typeof comp.view === "function" || typeof comp === "function")? comp : "div" attrs = params, currentPath = path, lastUpdate = null - render = (routeResolver.render || identity).bind(routeResolver) - redraw() + currentResolver = routeResolver.render ? routeResolver : null + if (init) redrawService.redraw() + else { + init = true + redrawService.redraw.sync() + } } if (payload.view || typeof payload === "function") update({}, payload) else { @@ -40,7 +46,12 @@ module.exports = function($window, redrawService) { } else update(payload, "div") } - }, bail, defaultRoute) + }, bail, defaultRoute, function (unsubscribe) { + redrawService.subscribe(root, function(sub) { + sub.c = run + return sub + }, unsubscribe) + }) } route.set = function(path, data, options) { if (lastUpdate != null) { diff --git a/api/tests/test-redraw.js b/api/tests/test-redraw.js index 0652f351..a04dbe84 100644 --- a/api/tests/test-redraw.js +++ b/api/tests/test-redraw.js @@ -38,15 +38,15 @@ o.spec("redrawService", function() { redrawService.subscribe(root, spy) - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) redrawService.redraw() - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) throttleMock.fire() - o(spy.callCount).equals(1) + o(spy.callCount).equals(2) }) o("should run a single renderer entry", function(done) { @@ -54,15 +54,15 @@ o.spec("redrawService", function() { redrawService.subscribe(root, spy) - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) redrawService.redraw() redrawService.redraw() redrawService.redraw() - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) setTimeout(function() { - o(spy.callCount).equals(1) + o(spy.callCount).equals(2) done() }, 20) @@ -82,57 +82,67 @@ o.spec("redrawService", function() { redrawService.redraw() - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) - o(spy3.callCount).equals(0) + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + o(spy3.callCount).equals(1) redrawService.redraw() - o(spy1.callCount).equals(0) - o(spy2.callCount).equals(0) - o(spy3.callCount).equals(0) + o(spy1.callCount).equals(1) + o(spy2.callCount).equals(1) + o(spy3.callCount).equals(1) setTimeout(function() { - o(spy1.callCount).equals(1) - o(spy2.callCount).equals(1) - o(spy3.callCount).equals(1) + o(spy1.callCount).equals(2) + o(spy2.callCount).equals(2) + o(spy3.callCount).equals(2) done() }, 20) }) o("should stop running after unsubscribe", function(done) { - var spy = o.spy(function() { - throw new Error("This shouldn't have been called") - }) + var spy = o.spy() redrawService.subscribe(root, spy) - redrawService.unsubscribe(root, spy) + o(spy.callCount).equals(1) + redrawService.unsubscribe(root) redrawService.redraw() - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) setTimeout(function() { - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) done() }, 20) }) + o("should invoke remove callback on unsubscribe", function() { + var spy = o.spy() + var onremove = o.spy() + + redrawService.subscribe(root, spy, onremove) + o(spy.callCount).equals(1) + redrawService.unsubscribe(root) + + o(spy.callCount).equals(1) + o(onremove.callCount).equals(1) + }) + 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") - }) + var spy = o.spy() redrawService.subscribe(root, spy) + o(spy.callCount).equals(1) redrawService.redraw() - redrawService.unsubscribe(root, spy) + redrawService.unsubscribe(root) - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) setTimeout(function() { - o(spy.callCount).equals(0) + o(spy.callCount).equals(1) done() }, 20) @@ -142,12 +152,13 @@ o.spec("redrawService", function() { var spy = o.spy() redrawService.subscribe(root, spy) - redrawService.unsubscribe(null) + o(spy.callCount).equals(1) + redrawService.unsubscribe(null) redrawService.redraw() setTimeout(function() { - o(spy.callCount).equals(1) + o(spy.callCount).equals(2) done() }, 20) @@ -165,12 +176,6 @@ o.spec("redrawService", function() { 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) @@ -180,5 +185,11 @@ o.spec("redrawService", function() { o(spy1.callCount).equals(2) o(spy2.callCount).equals(2) o(spy3.callCount).equals(2) + + redrawService.redraw.sync() + + o(spy1.callCount).equals(3) + o(spy2.callCount).equals(3) + o(spy3.callCount).equals(3) }) }) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index cf99e4c6..1f7fc1d9 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -111,6 +111,25 @@ o.spec("route", function() { o(view.callCount).equals(2) }) + o("subscribes correctly and removes when unmounted", function() { + $window.location.href = prefix + "/" + + route(root, "/", { + "/" : { + view: function() { + return m("div") + } + } + }) + + o(root.firstChild.nodeName).equals("DIV") + + // unsubscribe as if via `m.mount(root)` + redrawService.unsubscribe(root) + + o(root.childNodes.length).equals(0) + }) + o("default route doesn't break back button", function(done) { $window.location.href = "http://old.com" $window.location.href = "http://new.com" diff --git a/bundler/bundle.js b/bundler/bundle.js index 3a561d68..58ada442 100644 --- a/bundler/bundle.js +++ b/bundler/bundle.js @@ -15,6 +15,7 @@ function parse(file) { try {return JSON.parse(json)} catch (e) {throw new Error("invalid JSON: " + json)} } +var pkg = require("../package.json") var error module.exports = function (input) { var modules = {} @@ -23,6 +24,10 @@ module.exports = function (input) { var include = /(?:((?:var|let|const|,|)[\t ]*)([\w_$\.\[\]"'`]+)(\s*=\s*))?require\(([^\)]+)\)(\s*[`\.\(\[])?/gm var uuid = 0 var process = function(filepath, data) { + // HACK: inline Mithril's `package.json` keys without reading the whole file. + data = data.replace(/require\((['"])\.\/package\.json\1\)\.(\w+)/, function (match, quote, key) { + return JSON.stringify(pkg[key]) + }) data.replace(declaration, function(match, binding) {bindings[binding] = 0}) return data.replace(include, function(match, def, variable, eq, dep, rest) { @@ -106,13 +111,10 @@ module.exports = function (input) { + (rest ? "\n" + def + variable + eq + "_" + uuid : "") // if `rest` is truthy, it means the expression is fluent or higher-order (e.g. require(path).foo or require(path)(foo) } - var versionTag = "bleeding-edge" - var packageFile = __dirname + "/../package.json" var code = process(path.resolve(input), read(input)) .replace(/^\s*((?:var|let|const|)[\t ]*)([\w_$\.]+)(\s*=\s*)(\2)(?=[\s]+(\w)|;|$)/gm, "") // remove assignments to self .replace(/;+(\r|\n|$)/g, ";$1") // remove redundant semicolons .replace(/(\r|\n)+/g, "\n").replace(/(\r|\n)$/, "") // remove multiline breaks - .replace(versionTag, isFile(packageFile) ? parse(packageFile).version : versionTag) // set version code = ";(function() {\n" + code + "\n}());" //try {new Function(code); console.log("build completed at " + new Date())} catch (e) {} diff --git a/bundler/tests/test-bundler.js b/bundler/tests/test-bundler.js index 91da45ed..9c14ba82 100644 --- a/bundler/tests/test-bundler.js +++ b/bundler/tests/test-bundler.js @@ -4,6 +4,7 @@ var o = require("../../ospec/ospec") var bundle = require("../bundle") var fs = require("fs") +var pkg = require("../../package.json") var ns = "./" function write(filepath, data) { @@ -319,4 +320,11 @@ o.spec("bundler", function() { remove("a.js") remove("b.js") }) + o("reads package.json keys", function() { + write("a.js", 'var b = require("./package.json").version') + + o(bundle(ns + "a.js")).equals(";(function() {\nvar b = " + JSON.stringify(pkg.version) + "\n}());") + + remove("a.js") + }) }) diff --git a/docs/change-log.md b/docs/change-log.md index f4510f85..c59ea84c 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -78,6 +78,9 @@ - route: Declared routes in `m.route` now support `-` and `.` as delimiters for path segments. This means you can have a route like `"/edit/:file.:ext"`. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361)) - Previously, this was possible to do in `m.route.set`, `m.request`, and `m.jsonp`, but it was wholly untested for and also undocumented. - API: `m.buildPathname` and `m.parsePathname` added. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361)) +- route: Use `m.mount(root, null)` to unsubscribe and clean up after a `m.route(root, ...)` call. ([#2453](https://github.com/MithrilJS/mithril.js/pull/2453)) +- version: `m.version` returns the previous version string for what's in `next`. ([#2453](https://github.com/MithrilJS/mithril.js/pull/2453)) + - If you're using `next`, you should hopefully know what you're doing. If you need stability, don't use `next`. (This is also why I'm not labelling it as a breaking change.) #### Bug fixes @@ -111,6 +114,7 @@ - request: correct IE workaround for response type non-support ([#2449](https://github.com/MithrilJS/mithril.js/pull/2449) [@isiahmeadows](https://github.com/isiahmeadows)) - render: correct `contenteditable` check to also check for `contentEditable` property name ([#2450](https://github.com/MithrilJS/mithril.js/pull/2450) [@isiahmeadows](https://github.com/isiahmeadows)) - docs: clarify valid key usage ([#2452](https://github.com/MithrilJS/mithril.js/pull/2452) [@isiahmeadows](https://github.com/isiahmeadows)) +- route: don't pollute globals ([#2453](https://github.com/MithrilJS/mithril.js/pull/2453) [@isiahmeadows](https://github.com/isiahmeadows)) --- diff --git a/docs/route.md b/docs/route.md index 434ae457..4ec62e83 100644 --- a/docs/route.md +++ b/docs/route.md @@ -24,6 +24,7 @@ - [Authentication](#authentication) - [Preloading data](#preloading-data) - [Code splitting](#code-splitting) +- [Third-party integration](#third-party-integration) --- @@ -730,3 +731,81 @@ m.route(document.body, "/", { }, }) ``` + +--- + +### Third-party integration + +In certain situations, you may find yourself needing to interoperate with another framework like React. Here's how you do it: + +- Define all your routes using `m.route` as normal, but make sure you only use it *once*. Multiple route points are not supported. +- When you need to remove routing subscriptions, use `m.mount(root, null)`, using the same root you used `m.route(root, ...)` on. + +Here's an example with React: + +```javascript +class Child extends React.Component { + constructor(props) { + super(props) + this.root = React.createRef() + } + + componentDidMount() { + m.route(this.root, "/", { + // ... + }) + } + + componentDidUnmount() { + m.mount(this.root, null) + } + + render() { + return
+ } +} +``` + +And here's the rough equivalent with Vue: + +```html +
+``` + +```javascript +Vue.component("my-child", { + template: `
`, + mounted: function() { + m.route(this.$refs.root, "/", { + // ... + }) + }, + destroyed: function() { + m.mount(this.$refs.root, null) + }, +}) +``` + +Technically, there's nothing stopping you from even doing it in a Mithril component, even. + +```javascript +// Don't do this. Use a proper global layout component for each route instead, +// passing your child vnode/component in the attributes or children. +function Child() { + return { + oncreate: function(vnode) { + m.route(vnode.dom, "/", { + // ... + }) + }, + + onremove: function() { + m.mount(vnode.dom, null) + }, + + view: function() { + return m("div") + }, + } +} +``` diff --git a/index.js b/index.js index 8f4e71b2..1f775a5d 100644 --- a/index.js +++ b/index.js @@ -21,7 +21,7 @@ m.parseQueryString = require("./querystring/parse") m.buildQueryString = require("./querystring/build") m.parsePathname = require("./pathname/parse") m.buildPathname = require("./pathname/build") -m.version = "bleeding-edge" +m.version = require("./package.json").version m.vnode = require("./render/vnode") m.PromisePolyfill = require("./promise/polyfill") diff --git a/request/tests/test-request.js b/request/tests/test-request.js index 3b0990ba..2cc6b5c8 100644 --- a/request/tests/test-request.js +++ b/request/tests/test-request.js @@ -4,13 +4,13 @@ var o = require("../../ospec/ospec") var callAsync = require("../../test-utils/callAsync") var xhrMock = require("../../test-utils/xhrMock") var Request = require("../../request/request") -var Promise = require("../../promise/promise") +var PromisePolyfill = require("../../promise/promise") o.spec("request", function() { var mock, request, complete o.beforeEach(function() { mock = xhrMock() - var requestService = Request(mock, Promise) + var requestService = Request(mock, PromisePolyfill) request = requestService.request complete = o.spy() requestService.setCompletionCallback(complete) @@ -589,11 +589,13 @@ o.spec("request", function() { callAsync(function() { callAsync(function() { - o(catch1.callCount).equals(1) - o(then.callCount).equals(0) - o(catch2.callCount).equals(1) - o(catch3.callCount).equals(1) - done() + callAsync(function() { + o(catch1.callCount).equals(1) + o(then.callCount).equals(0) + o(catch2.callCount).equals(1) + o(catch3.callCount).equals(1) + done() + }) }) }) }) @@ -759,6 +761,14 @@ o.spec("request", function() { ) o("invokes the redraw in native async/await", function () { + // Use the native promise for correct semantics. This test will fail + // if you use the polyfill, as it's based on `setImmediate` (falling + // back to `setTimeout`), and promise microtasks are run at higher + // priority than either of those. + var requestService = Request(mock, Promise) + request = requestService.request + complete = o.spy() + requestService.setCompletionCallback(complete) mock.$defineRoutes({ "GET /item": function() { return {status: 200, responseText: "[]"} diff --git a/router/router.js b/router/router.js index e911c0a9..37fc29b5 100644 --- a/router/router.js +++ b/router/router.js @@ -6,91 +6,107 @@ var compileTemplate = require("../pathname/compileTemplate") var assign = require("../pathname/assign") module.exports = function($window) { - var supportsPushState = typeof $window.history.pushState === "function" var callAsync = typeof setImmediate === "function" ? setImmediate : setTimeout + var supportsPushState = typeof $window.history.pushState === "function" + var fireAsync - var asyncId - var router = {prefix: "#!"} - router.getPath = function() { - // Consider the pathname holistically. The prefix might even be invalid, - // but that's not our problem. - var prefix = $window.location.hash - if (router.prefix[0] !== "#") { - prefix = $window.location.search + prefix - if (router.prefix[0] !== "?") { - prefix = $window.location.pathname + prefix - if (prefix[0] !== "/") prefix = "/" + prefix + return { + prefix: "#!", + + getPath: function() { + // Consider the pathname holistically. The prefix might even be invalid, + // but that's not our problem. + var prefix = $window.location.hash + if (this.prefix[0] !== "#") { + prefix = $window.location.search + prefix + if (this.prefix[0] !== "?") { + prefix = $window.location.pathname + prefix + if (prefix[0] !== "/") prefix = "/" + prefix + } } - } - // This seemingly useless `.concat()` speeds up the tests quite a bit, - // since the representation is consistently a relatively poorly - // optimized cons string. - return prefix.concat() - .replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent) - .slice(router.prefix.length) - } + // This seemingly useless `.concat()` speeds up the tests quite a bit, + // since the representation is consistently a relatively poorly + // optimized cons string. + return prefix.concat() + .replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent) + .slice(this.prefix.length) + }, - router.setPath = function(path, data, options) { - path = buildPathname(path, data) - if (supportsPushState) { - var state = options ? options.state : null - var title = options ? options.title : null - $window.onpopstate() - if (options && options.replace) $window.history.replaceState(state, title, router.prefix + path) - else $window.history.pushState(state, title, router.prefix + path) - } - else $window.location.href = router.prefix + path - } - - router.defineRoutes = function(routes, resolve, reject, defaultRoute) { - var compiled = Object.keys(routes).map(function(route) { - if (route.charAt(0) !== "/") throw new SyntaxError("Routes must start with a `/`") - if ((/:([^\/\.-]+)(\.{3})?:/).test(route)) { - throw new SyntaxError("Route parameter names must be separated with either `/`, `.`, or `-`") + setPath: function(path, data, options) { + path = buildPathname(path, data) + if (fireAsync != null) { + fireAsync() + var state = options ? options.state : null + var title = options ? options.title : null + if (options && options.replace) $window.history.replaceState(state, title, this.prefix + path) + else $window.history.pushState(state, title, this.prefix + path) } - return { - route: route, - component: routes[route], - check: compileTemplate(route), + else { + $window.location.href = this.prefix + path } - }) + }, - if (defaultRoute != null) { - var defaultData = parsePathname(defaultRoute) + defineRoutes: function(routes, resolve, reject, defaultRoute, subscribe) { + var self = this + var compiled = Object.keys(routes).map(function(route) { + if (route[0] !== "/") throw new SyntaxError("Routes must start with a `/`") + if ((/:([^\/\.-]+)(\.{3})?:/).test(route)) { + throw new SyntaxError("Route parameter names must be separated with either `/`, `.`, or `-`") + } + return { + route: route, + component: routes[route], + check: compileTemplate(route), + } + }) + var unsubscribe, asyncId - if (!compiled.some(function (i) { return i.check(defaultData) })) { - throw new ReferenceError("Default route doesn't match any known routes") - } - } + fireAsync = null - function resolveRoute() { - var path = router.getPath() - var data = parsePathname(path) + if (defaultRoute != null) { + var defaultData = parsePathname(defaultRoute) - assign(data.params, $window.history.state) - - for (var i = 0; i < compiled.length; i++) { - if (compiled[i].check(data)) { - resolve(compiled[i].component, data.params, path, compiled[i].route) - return + if (!compiled.some(function (i) { return i.check(defaultData) })) { + throw new ReferenceError("Default route doesn't match any known routes") } } - reject(path, data.params) - } + function resolveRoute() { + var path = self.getPath() + var data = parsePathname(path) - if (supportsPushState) { - $window.onpopstate = function() { - if (asyncId) return - asyncId = callAsync(function() { - asyncId = null - resolveRoute() - }) + assign(data.params, $window.history.state) + + for (var i = 0; i < compiled.length; i++) { + if (compiled[i].check(data)) { + resolve(compiled[i].component, data.params, path, compiled[i].route) + return + } + } + + reject(path, data.params) } - } - else if (router.prefix.charAt(0) === "#") $window.onhashchange = resolveRoute - resolveRoute() - } - return router + if (supportsPushState) { + unsubscribe = function() { + $window.removeEventListener("popstate", fireAsync, false) + } + $window.addEventListener("popstate", fireAsync = function() { + if (asyncId) return + asyncId = callAsync(function() { + asyncId = null + resolveRoute() + }) + }, false) + } else if (this.prefix[0] === "#") { + unsubscribe = function() { + $window.removeEventListener("hashchange", resolveRoute, false) + } + $window.addEventListener("hashchange", resolveRoute, false) + } + + subscribe(unsubscribe) + resolveRoute() + }, + } } diff --git a/router/tests/test-defineRoutes.js b/router/tests/test-defineRoutes.js index b85b0137..39b85366 100644 --- a/router/tests/test-defineRoutes.js +++ b/router/tests/test-defineRoutes.js @@ -11,6 +11,10 @@ o.spec("Router.defineRoutes", function() { o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() { var $window, router, onRouteChange, onFail + function defineRoutes(routes, defaultRoute) { + router.defineRoutes(routes, onRouteChange, onFail, defaultRoute, function() {}) + } + o.beforeEach(function() { $window = pushStateMock(env) router = new Router($window) @@ -21,7 +25,10 @@ o.spec("Router.defineRoutes", function() { o("calls onRouteChange on init", function(done) { $window.location.href = prefix + "/a" - router.defineRoutes({"/a": {data: 1}}, onRouteChange, onFail) + var subscribe = o.spy() + + router.defineRoutes({"/a": {data: 1}}, onRouteChange, onFail, null, subscribe) + o(subscribe.callCount).equals(1) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -32,7 +39,7 @@ o.spec("Router.defineRoutes", function() { o("resolves to route", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -45,7 +52,7 @@ o.spec("Router.defineRoutes", function() { o("resolves to route w/ escaped unicode", function(done) { $window.location.href = prefix + "/%C3%B6?%C3%B6=%C3%B6" - router.defineRoutes({"/ö": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/ö": {data: 2}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -58,7 +65,7 @@ o.spec("Router.defineRoutes", function() { o("resolves to route w/ unicode", function(done) { $window.location.href = prefix + "/ö?ö=ö" - router.defineRoutes({"/ö": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/ö": {data: 2}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -75,7 +82,7 @@ o.spec("Router.defineRoutes", function() { router = new Router($window) router.prefix = prefix - router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -88,7 +95,7 @@ o.spec("Router.defineRoutes", function() { o("handles parameterized route", function(done) { $window.location.href = prefix + "/test/x" - router.defineRoutes({"/test/:a": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test/:a": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -101,7 +108,7 @@ o.spec("Router.defineRoutes", function() { o("handles multi-parameterized route", function(done) { $window.location.href = prefix + "/test/x/y" - router.defineRoutes({"/test/:a/:b": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test/:a/:b": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -114,7 +121,7 @@ o.spec("Router.defineRoutes", function() { o("handles rest parameterized route", function(done) { $window.location.href = prefix + "/test/x/y" - router.defineRoutes({"/test/:a...": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test/:a...": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -127,7 +134,7 @@ o.spec("Router.defineRoutes", function() { o("handles route with search", function(done) { $window.location.href = prefix + "/test?a=b&c=d" - router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -140,7 +147,7 @@ o.spec("Router.defineRoutes", function() { o("calls reject", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/other": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/other": {data: 1}}) callAsync(function() { o(onFail.callCount).equals(1) @@ -152,7 +159,7 @@ o.spec("Router.defineRoutes", function() { o("handles out of order routes", function(done) { $window.location.href = prefix + "/z/y/x" - router.defineRoutes({"/z/y/x": {data: 1}, "/:a...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/z/y/x": {data: 1}, "/:a...": {data: 2}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -164,7 +171,7 @@ o.spec("Router.defineRoutes", function() { o("handles reverse out of order routes", function(done) { $window.location.href = prefix + "/z/y/x" - router.defineRoutes({"/:a...": {data: 2}, "/z/y/x": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/:a...": {data: 2}, "/z/y/x": {data: 1}}) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -180,7 +187,7 @@ o.spec("Router.defineRoutes", function() { routes["/:a..."] = {data: 2} $window.location.href = prefix + "/z/y/x" - router.defineRoutes(routes, onRouteChange, onFail) + defineRoutes(routes) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -196,7 +203,7 @@ o.spec("Router.defineRoutes", function() { routes["/z/y/x"] = {data: 1} $window.location.href = prefix + "/z/y/x" - router.defineRoutes(routes, onRouteChange, onFail) + defineRoutes(routes) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -211,7 +218,7 @@ o.spec("Router.defineRoutes", function() { routes["/:a..."] = {data: 2} $window.location.href = prefix + "/z/y/x" - router.defineRoutes(routes, onRouteChange, onFail) + defineRoutes(routes) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -226,7 +233,7 @@ o.spec("Router.defineRoutes", function() { routes["/z/y/x"] = {data: 12} $window.location.href = prefix + "/z/y/x" - router.defineRoutes(routes, onRouteChange, onFail) + defineRoutes(routes) callAsync(function() { o(onRouteChange.callCount).equals(1) @@ -238,7 +245,7 @@ o.spec("Router.defineRoutes", function() { o("handles non-ascii routes", function(done) { $window.location.href = prefix + "/ö" - router.defineRoutes({"/ö": "aaa"}, onRouteChange, onFail) + defineRoutes({"/ö": "aaa"}) callAsync(function() { o(onRouteChange.callCount).equals(1) diff --git a/router/tests/test-getPath.js b/router/tests/test-getPath.js index c405923e..f79e133f 100644 --- a/router/tests/test-getPath.js +++ b/router/tests/test-getPath.js @@ -10,6 +10,10 @@ o.spec("Router.getPath", function() { o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() { var $window, router, onRouteChange, onFail + function defineRoutes(routes, defaultRoute) { + router.defineRoutes(routes, onRouteChange, onFail, defaultRoute, function() {}) + } + o.beforeEach(function() { $window = pushStateMock(env) router = new Router($window) @@ -20,25 +24,25 @@ o.spec("Router.getPath", function() { o("gets route", function() { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}}) o(router.getPath()).equals("/test") }) o("gets route w/ params", function() { $window.location.href = prefix + "/other/x/y/z?c=d#e=f" - router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}) o(router.getPath()).equals("/other/x/y/z?c=d#e=f") }) o("gets route w/ escaped unicode", function() { $window.location.href = prefix + "/%C3%B6?%C3%B6=%C3%B6#%C3%B6=%C3%B6" - router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}) o(router.getPath()).equals("/ö?ö=ö#ö=ö") }) o("gets route w/ unicode", function() { $window.location.href = prefix + "/ö?ö=ö#ö=ö" - router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}) o(router.getPath()).equals("/ö?ö=ö#ö=ö") }) diff --git a/router/tests/test-setPath.js b/router/tests/test-setPath.js index a76d3580..8447c512 100644 --- a/router/tests/test-setPath.js +++ b/router/tests/test-setPath.js @@ -11,6 +11,10 @@ o.spec("Router.setPath", function() { o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() { var $window, router, onRouteChange, onFail + function defineRoutes(routes, defaultRoute) { + router.defineRoutes(routes, onRouteChange, onFail, defaultRoute, function() {}) + } + o.beforeEach(function() { $window = pushStateMock(env) router = new Router($window) @@ -21,7 +25,7 @@ o.spec("Router.setPath", function() { o("setPath calls onRouteChange asynchronously", function(done) { $window.location.href = prefix + "/a" - router.defineRoutes({"/a": {data: 1}, "/b": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/a": {data: 1}, "/b": {data: 2}}) callAsync(function() { router.setPath("/b") @@ -35,7 +39,7 @@ o.spec("Router.setPath", function() { }) o("setPath calls onFail asynchronously", function(done) { $window.location.href = prefix + "/a" - router.defineRoutes({"/a": {data: 1}, "/b": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/a": {data: 1}, "/b": {data: 2}}) callAsync(function() { router.setPath("/c") @@ -49,7 +53,7 @@ o.spec("Router.setPath", function() { }) o("sets route via API", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}) callAsync(function() { router.setPath("/other/x/y/z?c=d#e=f") @@ -61,7 +65,7 @@ o.spec("Router.setPath", function() { }) o("sets route w/ escaped unicode", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}) callAsync(function() { router.setPath("/%C3%B6?%C3%B6=%C3%B6#%C3%B6=%C3%B6") @@ -73,7 +77,7 @@ o.spec("Router.setPath", function() { }) o("sets route w/ unicode", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}) callAsync(function() { router.setPath("/ö?ö=ö#ö=ö") @@ -90,7 +94,7 @@ o.spec("Router.setPath", function() { router = new Router($window) router.prefix = prefix - router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}) callAsync(function() { router.setPath("/other/x/y/z?c=d#e=f") @@ -102,7 +106,7 @@ o.spec("Router.setPath", function() { }) o("sets route via pushState/onpopstate", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}) callAsync(function() { $window.history.pushState(null, null, prefix + "/other/x/y/z?c=d#e=f") @@ -115,7 +119,7 @@ o.spec("Router.setPath", function() { }) o("sets parameterized route", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}) callAsync(function() { router.setPath("/other/:a/:b", {a: "x", b: "y/z", c: "d", e: "f"}) @@ -127,7 +131,7 @@ o.spec("Router.setPath", function() { }) o("replace:true works", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/other": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other": {data: 2}}) callAsync(function() { router.setPath("/other", null, {replace: true}) @@ -140,7 +144,7 @@ o.spec("Router.setPath", function() { }) o("replace:false works", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/other": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other": {data: 2}}) callAsync(function() { router.setPath("/other", null, {replace: false}) @@ -155,7 +159,7 @@ o.spec("Router.setPath", function() { }) o("state works", function(done) { $window.location.href = prefix + "/test" - router.defineRoutes({"/test": {data: 1}, "/other": {data: 2}}, onRouteChange, onFail) + defineRoutes({"/test": {data: 1}, "/other": {data: 2}}) callAsync(function() { router.setPath("/other", null, {state: {a: 1}}) diff --git a/test-utils/domMock.js b/test-utils/domMock.js index 840fda08..15a89e35 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -224,8 +224,16 @@ module.exports = function(options) { return string.replace(/-\D/g, function(match) {return match[1].toUpperCase()}) } var activeElement + var delay = 16, last = 0 var $window = { DOMParser: DOMParser, + requestAnimationFrame: function(callback) { + var elapsed = Date.now() - last + return setTimeout(function() { + callback() + last = Date.now() + }, delay - elapsed) + }, document: { createElement: function(tag) { var cssText = "" diff --git a/test-utils/pushStateMock.js b/test-utils/pushStateMock.js index f2273a2d..e65f79e7 100644 --- a/test-utils/pushStateMock.js +++ b/test-utils/pushStateMock.js @@ -187,5 +187,13 @@ module.exports = function(options) { $window.onhashchange = null, $window.onunload = null + $window.addEventListener = function (name, handler) { + $window["on" + name] = handler + } + + $window.removeEventListener = function (name, handler) { + $window["on" + name] = handler + } + return $window } diff --git a/tests/test-api.js b/tests/test-api.js index 1304542f..542322d8 100644 --- a/tests/test-api.js +++ b/tests/test-api.js @@ -5,12 +5,16 @@ var browserMock = require("../test-utils/browserMock") var components = require("../test-utils/components") o.spec("api", function() { - var m var FRAME_BUDGET = Math.floor(1000 / 60) - o.beforeEach(function() { - var mock = browserMock() - if (typeof global !== "undefined") global.window = mock - m = require("../mithril") // eslint-disable-line global-require + var mock = browserMock(), root + if (typeof global !== "undefined") { + global.window = mock + global.requestAnimationFrame = mock.requestAnimationFrame + } + var m = require("..") // eslint-disable-line global-require + + o.afterEach(function() { + if (root) m.mount(root, null) }) o.spec("m", function() { @@ -71,7 +75,7 @@ o.spec("api", function() { }) o.spec("m.render", function() { o("works", function() { - var root = window.document.createElement("div") + root = window.document.createElement("div") m.render(root, m("div")) o(root.childNodes.length).equals(1) @@ -84,7 +88,7 @@ o.spec("api", function() { o.spec("m.mount", function() { o("works", function() { - var root = window.document.createElement("div") + root = window.document.createElement("div") m.mount(root, createComponent({view: function() {return m("div")}})) o(root.childNodes.length).equals(1) @@ -93,7 +97,7 @@ o.spec("api", function() { }) o.spec("m.route", function() { o("works", function(done) { - var root = window.document.createElement("div") + root = window.document.createElement("div") m.route(root, "/a", { "/a": createComponent({view: function() {return m("div")}}) }) @@ -106,7 +110,7 @@ o.spec("api", function() { }, FRAME_BUDGET) }) o("m.route.prefix", function(done) { - var root = window.document.createElement("div") + root = window.document.createElement("div") m.route.prefix("#") m.route(root, "/a", { "/a": createComponent({view: function() {return m("div")}}) @@ -120,7 +124,7 @@ o.spec("api", function() { }, FRAME_BUDGET) }) o("m.route.get", function(done) { - var root = window.document.createElement("div") + root = window.document.createElement("div") m.route(root, "/a", { "/a": createComponent({view: function() {return m("div")}}) }) @@ -133,7 +137,7 @@ o.spec("api", function() { }) o("m.route.set", function(done, timeout) { timeout(100) - var root = window.document.createElement("div") + root = window.document.createElement("div") m.route(root, "/a", { "/:id": createComponent({view: function() {return m("div")}}) }) @@ -151,7 +155,7 @@ o.spec("api", function() { o.spec("m.redraw", function() { o("works", function(done) { var count = 0 - var root = window.document.createElement("div") + root = window.document.createElement("div") m.mount(root, createComponent({view: function() {count++}})) o(count).equals(1) m.redraw() @@ -164,7 +168,7 @@ o.spec("api", function() { }, FRAME_BUDGET) }) o("sync", function() { - var root = window.document.createElement("div") + root = window.document.createElement("div") var view = o.spy() m.mount(root, createComponent({view: view})) o(view.callCount).equals(1)