From 53a83a58f2bf46b1d63dfab1449a623ac89e92fa Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Tue, 23 Aug 2016 22:33:18 -0400 Subject: [PATCH] rename RouteResolver::view back to render to restore diff semantics prevent double resolving --- api/router.js | 16 +++-- api/tests/test-router.js | 125 +++++++++++++++++++++++++++++++-- docs/route.md | 63 +++++++++-------- render/render.js | 3 +- render/tests/test-component.js | 8 +-- util/tests/test-stream.js | 18 +++++ 6 files changed, 185 insertions(+), 48 deletions(-) diff --git a/api/router.js b/api/router.js index 0902da10..cbf4ab4d 100644 --- a/api/router.js +++ b/api/router.js @@ -9,13 +9,17 @@ module.exports = function($window, renderer, pubsub) { var route = function(root, defaultRoute, routes) { var current = {path: null, component: "div"} var replay = router.defineRoutes(routes, function(payload, args, path, route) { + var resolved = false + function resolve(component) { + if (resolved) return + resolved = true + current.path = path, current.component = component + renderer.render(root, payload.render(Vnode(component, null, args, undefined, undefined, undefined))) + } args.path = path, args.route = route - if (typeof payload.onmatch === "function") { - if (typeof payload.view !== "function") payload.view = function(vnode) {return vnode} - var resolve = function(component) { - current.path = path, current.component = component - renderer.render(root, payload.view(Vnode(component, null, args, undefined, undefined, undefined))) - } + if (typeof payload.view !== "function") { + if (typeof payload.render !== "function") payload.render = function(vnode) {return vnode} + if (typeof payload.onmatch !== "function") payload.onmatch = function() {resolve(current.component)} if (path !== current.path) payload.onmatch(Vnode(payload, null, args, undefined, undefined, undefined), resolve) else resolve(current.component) } diff --git a/api/tests/test-router.js b/api/tests/test-router.js index a3867884..dd47528a 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -252,7 +252,7 @@ o.spec("route", function() { resolve(Component) }, - view: function(vnode) { + render: function(vnode) { renderCount++ o(vnode.attrs.id).equals("abc") @@ -271,7 +271,7 @@ o.spec("route", function() { }, FRAME_BUDGET) }) - o("accepts RouteResolver without `view` method as payload", function(done) { + o("accepts RouteResolver without `render` method as payload", function(done) { var matchCount = 0 var Component = { view: function() { @@ -303,7 +303,7 @@ o.spec("route", function() { }, FRAME_BUDGET) }) - o("object without `onmatch` method acts as component", function(done) { + o("accepts RouteResolver without `onmatch` method as payload", function(done) { var renderCount = 0 var Component = { view: function() { @@ -314,7 +314,7 @@ o.spec("route", function() { $window.location.href = prefix + "/" route(root, "/abc", { "/:id" : { - view: function(vnode) { + render: function(vnode) { renderCount++ o(vnode.attrs.id).equals("abc") @@ -330,6 +330,44 @@ o.spec("route", function() { done() }, FRAME_BUDGET) }) + + o("RouteResolver `render` does not have component semantics", function(done, timeout) { + timeout(60) + + var renderCount = 0 + var A = { + view: function() { + return m("div") + } + } + + $window.location.href = prefix + "/" + route(root, "/a", { + "/a" : { + render: function(vnode) { + return m("div") + }, + }, + "/b" : { + render: function(vnode) { + return m("div") + }, + }, + }) + + setTimeout(function() { + var dom = root.firstChild + o(root.firstChild.nodeName).equals("DIV") + + route.set("/b") + + setTimeout(function() { + o(root.firstChild).equals(dom) + + done() + }, FRAME_BUDGET) + }, FRAME_BUDGET) + }) o("calls onmatch and view correct number of times", function(done) { var matchCount = 0 @@ -347,7 +385,7 @@ o.spec("route", function() { matchCount++ resolve(Component) }, - view: function(vnode) { + render: function(vnode) { renderCount++ return vnode }, @@ -368,6 +406,83 @@ o.spec("route", function() { }, FRAME_BUDGET) }) }) + + o("onmatch can redirect to another route", function(done) { + var redirected = false + + $window.location.href = prefix + "/" + route(root, "/a", { + "/a" : { + onmatch: function() { + route.set("/b") + } + }, + "/b" : { + view: function(vnode){ + redirected = true + } + } + }) + + setTimeout(function() { + o(redirected).equals(true) + + done() + }, FRAME_BUDGET) + }) + + o("onmatch can redirect to another route that has RouteResolver", function(done) { + var redirected = false + + $window.location.href = prefix + "/" + route(root, "/a", { + "/a" : { + onmatch: function() { + route.set("/b") + } + }, + "/b" : { + render: function(vnode){ + redirected = true + } + } + }) + + setTimeout(function() { + o(redirected).equals(true) + + done() + }, FRAME_BUDGET) + }) + + o("onmatch resolution callback resolves at most once", function(done) { + var resolveCount = 0 + var resolvedComponent + var A = {view: function() {}} + var B = {view: function() {}} + var C = {view: function() {}} + + $window.location.href = prefix + "/" + route(root, "/", { + "/" : { + onmatch: function(vnode, resolve) { + resolve(A) + resolve(B) + callAsync(function() {resolve(C)}) + }, + render: function(vnode) { + resolveCount++ + resolvedComponent = vnode.tag + } + }, + }) + setTimeout(function() { + o(resolveCount).equals(1) + o(resolvedComponent).equals(A) + + done() + }, FRAME_BUDGET) + }) }) }) }) diff --git a/docs/route.md b/docs/route.md index 60d59525..fd957f17 100644 --- a/docs/route.md +++ b/docs/route.md @@ -8,14 +8,14 @@ - [route.link](#route-link) - [RouteResolver](#routeresolver) - [routeResolver.onmatch](#routeresolver-onmatch) - - [routeResolver.view](#routeresolver-view) + - [routeResolver.render](#routeresolver-render) - [How it works](#how-it-works) - [Typical usage](#typical-usage) - [Navigating to different routes](#navigating-to-different-routes) - [Routing parameters](#routing-parameters) - [Changing router prefix](#changing-router-prefix) -- [Wrapping a layout component](#wrapping-a-layout-component) - [Advanced component resolution](#advanced-component-resolution) +- [Wrapping a layout component](#wrapping-a-layout-component) - [Authentication](#authentication) - [Code splitting](#code-splitting) @@ -82,9 +82,9 @@ Argument | Type | Required | Description #### RouteResolver -A RouterResolver is an object that contains an `onmatch` method, and optionally a `view` method. +A RouterResolver is an object that contains an `onmatch` method and/or a `render` method. Both methods are optional, but at least one must be present. -`routeResolver = {onmatch, view}` +`routeResolver = {onmatch, render}` ##### routeResolver.onmatch @@ -103,11 +103,11 @@ Argument | Type | Description `resolve` | `Function(Component)` | Call this function with a component as the first argument to use it as the route's component **returns** | | Returns `undefined` -##### routeResolver.view +##### routeResolver.render -The `view` method is called on every redraw for a matching route. It is similar to the `view` method in components and it exists to simplify [component composition](#wrapping-a-layout-component). +The `render` method is called on every redraw for a matching route. It is similar to the `view` method in components and it exists to simplify [component composition](#wrapping-a-layout-component). -`vnode = routeResolve.view(vnode)` +`vnode = routeResolve.render(vnode)` Argument | Type | Description ------------------- | --------------- | ----------- @@ -205,7 +205,6 @@ You can also navigate programmatically, via `m.route.set(route)`. For example, ` When navigating to routes, there's no need to explicitly specify the router prefix. In other words, don't add the hashbang `#!` in front of the route path when linking via `m.route.link` or redirecting. - --- ### Routing parameters @@ -230,6 +229,8 @@ In the example above, we defined a route `/edit/:id`. This creates a dynamic rou It's possible to have multiple arguments in a route, for example `/edit/:projectID/:userID` would yield the properties `projectID` and `userID` on the component's vnode attributes object. +In addition to routing parameters, the `attrs` object also includes a `path` property that contains the current route path, and a `route` property that contains the matched routed. + #### Variadic routes It's also possible to have variadic routes, i.e. a route with an argument that contains URL pathnames that contain slashes: @@ -263,28 +264,6 @@ m.route.prefix("/my-app") --- -### Wrapping a layout component - -You can use anonymous components to wrap a layout around a component, or to pass parameters to a top level component - -```javascript -var Layout = { - view: function(vnode) { - return m(".layout", vnode.children) - } -} - -m.route(document.body, "/", { - "/": { - view: function() { - return m(Layout, Home) - }, - } -}) -``` - ---- - ### Advanced component resolution Instead of mapping a component to a route, you can specify a RouteResolver object. A RouteResolver object contains a `onmatch()` method and a optionally a `view()` method. @@ -295,7 +274,7 @@ m.route(document.body, "/", { onmatch: function(vnode, resolve) { resolve(Home) }, - view: function(vnode) { + render: function(vnode) { return vnode }, } @@ -306,6 +285,28 @@ RouteResolvers are useful for implementing a variety of advanced routing use cas --- +### Wrapping a layout component + +You can use a RouteResolver to wrap a layout around a component, or to pass parameters to a top level component + +```javascript +var Layout = { + view: function(vnode) { + return m(".layout", vnode.children) + } +} + +m.route(document.body, "/", { + "/": { + render: function() { + return m(Layout, m(Home)) + }, + } +}) +``` + +--- + ### Authentication The RouterResolver's `onmatch` hook can be used to run logic before the top level component in a route is initializated. The example below shows how to implement a login wall that prevents users from seeing the `/secret` page unless they login. diff --git a/render/render.js b/render/render.js index 8c19c75d..b0091e41 100644 --- a/render/render.js +++ b/render/render.js @@ -98,8 +98,7 @@ module.exports = function($window) { initLifecycle(vnode.tag, vnode, hooks) vnode.instance = Vnode.normalize(vnode.tag.view.call(vnode.state, vnode)) if (vnode.instance != null) { - if(vnode.instance === vnode) - throw Error("A component view mustn't return the vnode that was supplied to it.") + if (vnode.instance === vnode) throw Error("A view cannot return the vnode it received as arguments") var element = createNode(vnode.instance, hooks, ns) vnode.dom = vnode.instance.dom vnode.domSize = vnode.dom != null ? vnode.instance.domSize : 0 diff --git a/render/tests/test-component.js b/render/tests/test-component.js index 13eb0bcc..d6d26e43 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -264,10 +264,10 @@ o.spec("component", function() { try { render(root, [{tag: component}]) } - catch(error){ - o(error instanceof Error).equals(true) - // Call stack excession is a RangeError - o(error instanceof RangeError).equals(false) + catch (e) { + o(e instanceof Error).equals(true) + // Call stack exception is a RangeError + o(e instanceof RangeError).equals(false) } }) o("can update when returning fragments", function() { diff --git a/util/tests/test-stream.js b/util/tests/test-stream.js index f6fe70c7..bd187d77 100644 --- a/util/tests/test-stream.js +++ b/util/tests/test-stream.js @@ -196,6 +196,24 @@ o.spec("stream", function() { straggler(30) o(all()).deepEquals([10, "20", 30]) }) + o("calls run callback after all parents are active", function() { + var value = 0 + var id = function(value) {return value} + var a = Stream() + var b = Stream() + + var all = Stream.merge([a.run(id).catch(id), b.run(id).catch(id)]).run(function(data) { + value = data[0] + data[1] + }) + + a(1) + b(2) + o(value).equals(3) + + a(3) + b(4) + o(value).equals(7) + }) }) o.spec("end", function() { o("end stream works", function() {