vndb/mithril-vndb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make errors and their messages more accurate and helpful (#2536)

Browse source 
Also, I normalized them to all be sentences for consistency, and I moved
the reentrancy check from `m.mount` to `m.render` to be a little more
helpful. The router change during mounting is inconsequential and only
to avoid the new modified error, and the change to the update loop is to
send the original error if an error occurred while initializing the
default route. (This is all around more useful anyways.)

And while I was at it, I fixed an obscure bug with sync redraws.
This commit is contained in:
Isiah Meadows 2019-09-30 16:08:04 -04:00 • committed by GitHub
parent 475747800a
commit b98ab29efd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 310 additions and 54 deletions
Download patch file Download diff file Expand all files Collapse all files

15
api/mount-redraw.js
View file

@ -4,17 +4,15 @@ var Vnode = require("../render/vnode")
module.exports = function(render, schedule, console) {
var subscriptions = []
var rendering = false
var pending = false
var offset = -1
function sync() {
if (rendering) throw new Error("Nested m.redraw.sync() call")
rendering = true
for (var i = 0; i < subscriptions.length; i += 2) {
try { render(subscriptions[i], Vnode(subscriptions[i + 1]), redraw) }
for (offset = 0; offset < subscriptions.length; offset += 2) {
try { render(subscriptions[offset], Vnode(subscriptions[offset + 1]), redraw) }
catch (e) { console.error(e) }
}
rendering = false
offset = -1
}
function redraw() {
@ -31,13 +29,14 @@ module.exports = function(render, schedule, console) {
function mount(root, component) {
if (component != null && component.view == null && typeof component !== "function") {
throw new TypeError("m.mount(element, component) expects a component, not a vnode")
throw new TypeError("m.mount expects a component, not a vnode.")
}
var index = subscriptions.indexOf(root)
if (index >= 0) {
subscriptions.splice(index, 2)
render(root, [], redraw)
if (index <= offset) offset -= 2
render(root, [])
}
if (component != null) {

24
api/router.js
View file

@ -33,16 +33,16 @@ module.exports = function($window, mountRedraw) {
var SKIP = route.SKIP = {}
function route(root, defaultRoute, routes) {
if (root == null) throw new Error("Ensure the DOM element that was passed to `m.route` is not undefined")
if (!root) throw new TypeError("DOM element being rendered to does not exist.")
// 0 = start
// 1 = init
// 2 = ready
var state = 0
var compiled = Object.keys(routes).map(function(route) {
if (route[0] !== "/") throw new SyntaxError("Routes must start with a `/`")
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 `-`")
throw new SyntaxError("Route parameter names must be separated with either '/', '.', or '-'.")
}
return {
route: route,
@ -61,7 +61,7 @@ module.exports = function($window, mountRedraw) {
var defaultData = parsePathname(defaultRoute)
if (!compiled.some(function (i) { return i.check(defaultData) })) {
throw new ReferenceError("Default route doesn't match any known routes")
throw new ReferenceError("Default route doesn't match any known routes.")
}
}
@ -87,8 +87,8 @@ module.exports = function($window, mountRedraw) {
assign(data.params, $window.history.state)
function fail() {
if (path === defaultRoute) throw new Error("Could not resolve default route " + defaultRoute)
function reject(e) {
console.error(e)
setPath(defaultRoute, null, {replace: true})
}
@ -123,13 +123,17 @@ module.exports = function($window, mountRedraw) {
else if (payload.onmatch) {
p.then(function () {
return payload.onmatch(data.params, path, matchedRoute)
}).then(update, fail)
}).then(update, path === defaultRoute ? null : reject)
}
else update("div")
return
}
}
fail()
if (path === defaultRoute) {
throw new Error("Could not resolve default route " + defaultRoute + ".")
}
setPath(defaultRoute, null, {replace: true})
}
}
@ -157,12 +161,11 @@ module.exports = function($window, mountRedraw) {
$window.addEventListener("hashchange", resolveRoute, false)
}
return mountRedraw.mount(root, {
mountRedraw.mount(root, {
onbeforeupdate: function() {
state = state ? 2 : 1
return !(!state || sentinel === currentResolver)
},
oncreate: resolveRoute,
onremove: onremove,
view: function() {
if (!state || sentinel === currentResolver) return
@ -172,6 +175,7 @@ module.exports = function($window, mountRedraw) {
return vnode
},
})
resolveRoute()
}
route.set = function(path, data, options) {
if (lastUpdate != null) {

154
api/tests/test-mountRedraw.js
View file

@ -91,6 +91,30 @@ o.spec("mount/redraw", function() {
o(spy3.callCount).equals(2)
})
o("should not redraw when mounting another root", 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()
m.mount(el1, {view: spy1})
o(spy1.callCount).equals(1)
o(spy2.callCount).equals(0)
o(spy3.callCount).equals(0)
m.mount(el2, {view: spy2})
o(spy1.callCount).equals(1)
o(spy2.callCount).equals(1)
o(spy3.callCount).equals(0)
m.mount(el3, {view: spy3})
o(spy1.callCount).equals(1)
o(spy2.callCount).equals(1)
o(spy3.callCount).equals(1)
})
o("should stop running after mount null", function() {
var spy = o.spy()
@ -204,6 +228,136 @@ o.spec("mount/redraw", function() {
o(function() { m.mount(root, {}) }).throws(TypeError)
})
o("skips roots that were synchronously unsubscribed before they were visited", function() {
var calls = []
var root1 = $document.createElement("div")
var root2 = $document.createElement("div")
var root3 = $document.createElement("div")
m.mount(root1, {
onbeforeupdate: function() {
m.mount(root2, null)
},
view: function() { calls.push("root1") },
})
m.mount(root2, {view: function() { calls.push("root2") }})
m.mount(root3, {view: function() { calls.push("root3") }})
o(calls).deepEquals([
"root1", "root2", "root3",
])
m.redraw.sync()
o(calls).deepEquals([
"root1", "root2", "root3",
"root1", "root3",
])
})
o("keeps its place when synchronously unsubscribing previously visited roots", function() {
var calls = []
var root1 = $document.createElement("div")
var root2 = $document.createElement("div")
var root3 = $document.createElement("div")
m.mount(root1, {view: function() { calls.push("root1") }})
m.mount(root2, {
onbeforeupdate: function() {
m.mount(root1, null)
},
view: function() { calls.push("root2") },
})
m.mount(root3, {view: function() { calls.push("root3") }})
o(calls).deepEquals([
"root1", "root2", "root3",
])
m.redraw.sync()
o(calls).deepEquals([
"root1", "root2", "root3",
"root1", "root2", "root3",
])
})
o("keeps its place when synchronously unsubscribing previously visited roots in the face of errors", function() {
errors = ["fail"]
var calls = []
var root1 = $document.createElement("div")
var root2 = $document.createElement("div")
var root3 = $document.createElement("div")
m.mount(root1, {view: function() { calls.push("root1") }})
m.mount(root2, {
onbeforeupdate: function() {
m.mount(root1, null)
throw "fail"
},
view: function() { calls.push("root2") },
})
m.mount(root3, {view: function() { calls.push("root3") }})
o(calls).deepEquals([
"root1", "root2", "root3",
])
m.redraw.sync()
o(calls).deepEquals([
"root1", "root2", "root3",
"root1", "root3",
])
})
o("keeps its place when synchronously unsubscribing the current root", function() {
var calls = []
var root1 = $document.createElement("div")
var root2 = $document.createElement("div")
var root3 = $document.createElement("div")
m.mount(root1, {view: function() { calls.push("root1") }})
m.mount(root2, {
onbeforeupdate: function() {
try { m.mount(root2, null) } catch (e) { calls.push([e.constructor, e.message]) }
},
view: function() { calls.push("root2") },
})
m.mount(root3, {view: function() { calls.push("root3") }})
o(calls).deepEquals([
"root1", "root2", "root3",
])
m.redraw.sync()
o(calls).deepEquals([
"root1", "root2", "root3",
"root1", [TypeError, "Node is currently being rendered to and thus is locked."], "root2", "root3",
])
})
o("keeps its place when synchronously unsubscribing the current root in the face of an error", function() {
errors = [
[TypeError, "Node is currently being rendered to and thus is locked."],
]
var calls = []
var root1 = $document.createElement("div")
var root2 = $document.createElement("div")
var root3 = $document.createElement("div")
m.mount(root1, {view: function() { calls.push("root1") }})
m.mount(root2, {
onbeforeupdate: function() {
try { m.mount(root2, null) } catch (e) { throw [e.constructor, e.message] }
},
view: function() { calls.push("root2") },
})
m.mount(root3, {view: function() { calls.push("root3") }})
o(calls).deepEquals([
"root1", "root2", "root3",
])
m.redraw.sync()
o(calls).deepEquals([
"root1", "root2", "root3",
"root1", "root3",
])
})
components.forEach(function(cmp){
o.spec(cmp.kind, function(){
var createComponent = cmp.create

14
api/tests/test-router.js
View file

@ -69,6 +69,9 @@ o.spec("route", function() {
}
}
// In case it doesn't get reset
var realError = console.error
o.beforeEach(function() {
currentTest = nextID++
$window = browserMock(env)
@ -79,11 +82,16 @@ o.spec("route", function() {
mountRedraw = apiMountRedraw(coreRenderer($window), throttleMock.schedule, console)
route = apiRouter($window, mountRedraw)
route.prefix = prefix
console.error = function() {
realError.call(this, new Error("Unexpected `console.error` call"))
realError.apply(this, arguments)
}
})
o.afterEach(function() {
o(throttleMock.queueLength()).equals(0)
currentTest = -1 // doesn't match any test
console.error = realError
})
o("throws on invalid `root` DOM node", function() {
@ -1082,11 +1090,13 @@ o.spec("route", function() {
var matchCount = 0
var renderCount = 0
var spy = o.spy()
var error = new Error("error")
var errorSpy = console.error = o.spy()
var resolver = {
onmatch: lock(function() {
matchCount++
return Promise.reject(new Error("error"))
return Promise.reject(error)
}),
render: lock(function(vnode) {
renderCount++
@ -1104,6 +1114,8 @@ o.spec("route", function() {
o(matchCount).equals(1)
o(renderCount).equals(0)
o(spy.callCount).equals(1)
o(errorSpy.callCount).equals(1)
o(errorSpy.args[0]).equals(error)
})
})