[rewrite] Rest of the API

[dead-claudia]

Since we're starting to get to that point, I think it's a good idea to start thinking of the rest of the high-level API. Here's some of my ideas:

• Mithril.m(type, attrs?, ...children)

  The familiar m() hyperscript API.

• var trust = Mithril.trust

• var mountpoint = Mithril.mount(window = global.window)

  Ideally, this should return an object you can use to do m.redraw()-like things and have multiple independent roots. The argument is for easy mocking, and effectively nullifies m.deps.

• mountpoint.window

  This is the window value argument passed to Mithril.mount(), defaulting to the global window. You can also use a JSDOM window if you want to use it in Node.

• var promise = mountpoint.redraw()

  This should do the equivalent of 0.2's async m.redraw(), but return a Promise.

• mountpoint.redrawSync()

  This should do the equivalent of 0.2's sync m.redraw(true), and return nothing.

• mountpoint.redrawStrategy = "all" | "diff" | "none"

  This fufulls the same purpose as m.redraw.strategy(). It might not be necessary with the lifecycle methods, though.

• var promise = mountpoint.compute(callback: () => Promise | void)

  This should take the place of m.startComputation and m.endComputation. It is roughly equivalent to the following code:

  m.startComputation()
  return Promise.resolve(callback()).then(() => {
      m.endComputation()
  })

• Mithril.prop(init?, observer?)

  m.prop is popular enough that it should probably stay in core. I do believe it should lose the Promise absorbing semantics, though, and it should be optionally observable.

• Mithril.withAttr(value, callback, thisArg)

  m.withAttr also seems popular enough that it could remain in core, with few modifications (mostly removing compat bloat).

---

The rest are generally not necessary/needed in the core distribution.

• m.route can be implemented on top of m.mount in a third-party module.
• m.component can (and should) die.
• m.request should be go away in favor of the native Fetch API, and there is an XHR-based polyfill if you need one. IMHO, redraw after request should be explicit, anyways.
• m.deferred and m.sync should go away in favor of native Promises. If necessary, there's a ton of libraries and polyfills for that.

/cc @tivac @lhorie

[tivac]

Given the amount of 0.2.x code we have I'm much more in favor of an API that keeps most of the broad strokes of the 0.2.x API intact.

• I don't think the arguments to m.mount() should change
• I don't think that m.route() should be dropped
• I don't think that m.request() should be dropped (to ease transition, long-term I agree that fetch is a better plan)
• m.component could just be an alias for m() with a warning (the semantics are slightly different, but that'd be fine for our code at least)
• Re-render isn't tied to rAF loops any more, so I don't think that differentiating sync/async redraws makes a ton of sense.

I'm working on a very rough-n-tumble version of what I think this should look like in a branch.

[tinchoz49]

@isiahmeadows Maybe is a stupid question haha but just to be sure.
Represent the API with Mithril.m, Mithril.trust... is only an abstract representation right? I mean with the rewrite we are going to use a modular approach or am i wrong?

for example:

var m = require("mithril/render/hyperscript");

[dead-claudia]

@tivac

• I can see what you mean by m.render. IMO in this case it is best either always sync or always async (without even a toggle). That also nullifies m.redraw, m.compute, etc.
• m.request and m.component can be inserted in a compatibility wrapper apart from core, just concatenated with the main output.
• Would you be fine if m.route and m.request were made into separate modules that depended on Mithril instead? The latter could almost be ripped out now.

As for multiple mount points, we could instead do this:

• m.mount(element, component?, window = window) would do the standard global mount. It would also return the global mount point.
• m.mount() would get the global mount point. (An element is always required for creating mount points.)
• m.localMount(element, component?, window = window) would do basically what I initially suggested, optionally initializing it with a component.

For remounting, there are these properties/methods:

• mountpoint.element is the element it was initialized with.
• mountpoint.component is the component it was initialized with.
• mountpoint.initialize(component) would initialize the mountpoint with the component, and Mithril.localMount(window, element, component) is the same as m.localMount(window, element).initialize(component).
• mountpoint.clear() would be morally equivalent to m.mount(element, null).

The old API could be roughly replicated with this (the breaking part is that it doesn't return the vdom state):

function mount(root, component) {
    if (component == null) {
        m.mount().clear()
    } else {
        m.mount(root, component)
    }
}

[dead-claudia]

@tinchoz49

Currently, even though it's been modularized, and you will have the option of requiring individual parts, there will be a global build of everything and just specific parts. This issue is mostly focused on starting down that direction, since not much thought has yet to be put in it.

[tinchoz49]

ahhh ok!! Thank you for explaining to me 😌 !

[lhorie]

FYI, I started doing some work on exposing a 0.2.x-like API to help @tivac test.

Currently there's m, render, trust, a rough mount, route, request and redraw and a version of threadit.js running from the bundle. It pretty much has the same API except for config: m.route and the new state / lifecycle API.

I'll push it when I get back to my desktop so you guys can take a look

There's only one issue that I would consider major wrt post-request redraws, that I need to do some work on

[lhorie]

@tivac https://github.com/lhorie/mithril.js/blob/rewrite/examples/threaditjs-bundle/app.js

The mountpoint now looks like 0.2.x: https://github.com/lhorie/mithril.js/blob/rewrite/examples/threaditjs-bundle/app.js#L178 and post-request redraws still need to be called manually (but the m.redraw hook does work) https://github.com/lhorie/mithril.js/blob/rewrite/examples/threaditjs-bundle/app.js#L34

The only other caveat is that currently, config: m.route is not overloaded, and instead it uses a submethod called m.route.link (and config itself is replaced with oncreate) https://github.com/lhorie/mithril.js/blob/rewrite/examples/threaditjs-bundle/app.js#L91

The bundle can be regenerated by running node bundler/bundler.js. There are three files of interest that are bundle-specific: ./router.js and ./mount.js expose familiar interfaces to their respective modules, and ./index.js that glues everything together. Bundled data is output to the ./mithril.js file

Oh, and now, this points to vnode.state within lifecycle methods

[tivac]

@lhorie Wow, that's... comprehensive. I'll try to dig into it some this weekend!

[Naddiseo]

I agree with @tivac wrt m.request(), and it shouldn't be dropped until native fetch implementations are cancellable and support the streaming API. (We currently have a file upload component written in mithril which has a progress bar, which uses xhr.onprogress via m.request, so removing m.request will require us to have yet another way to send data to the server.)

Other thoughts:

• I like the promise based API.
• Will redrawStrategy still be a prop?
• Personally, I like, and use, the promise absorbing semantics of m.prop, what would replace it if it were removed?
• The m.prop observer would have to not incur a performance penalty if not used.
• I'm +0 on m.component removal
• +1 on m.deferred and m.sync removal

I say all this, but I think I'd need to actually try writing something using the new API/code to have a more informed view-point.

[tivac]

@lhorie Finally got a chance to play w/ the compat shim, thanks for that. Yours is much cleaner than my attempt!

1. It works! (for the simple app I ported)
2. Redraws aren't capped at 60fps via rAF any more, which means that if you redraw on scroll (like for occlusion culling a huge list) you end up w/ a lot of redraws stomping on each other & blank UI.
3. There's no way to cancel a redraw from an event. We chatted in gitter about options last week, but just bringing it up again as something I'd like to be able to do.
4. No surprise that it feels really, really fast. This is delightful.

[lhorie]

@tivac Awesome, glad to hear that!

Items 2) and 3) are still in the list of things to do. I'm specially open to suggestions re: cancelling redraws.

The two options on the table currently are:

• reimplement m.redraw.strategy
• prevent redraw if the e object has a specific flag, e.g. e.skipRedraw = true

The second option could be abstracted w/ a function, e.g. m.skipRedraw(e), to make it composable with e.g. .pipe in ramda

[tivac]

Based on anecdotal evidence from gitter/GH I think m.redraw.strategy() was a layer of abstraction that a lot of folks had trouble grokking. Setting a property on the event object seems clear and concise. A helper method to set it in a functional way isn't something I'd use but I don't have an issue with it.

e.skipRedraw = true seems perilously close to a boolean trap, but off the top of my head I'd advocate for e.redraw = false and I'm not sure that'd be any better.

[tivac]

Just pushed #1047 as my attempt to address the over-redrawing issue.

[tivac]

And #1049 to allow for skipping a re-draw via event handlers. Went w/ my redraw = false proposal, but easy to change to something else if preferred.

All of this API stuff needs some sort of tests though, @lhorie is that something you want to figure out or should I take a crack at it?

[lhorie]

@tivac I'm taking a crack at writing some tests for the limiter right now. I sometimes see redraws firing before 16ms in your scrolling test in Chrome, need to figure out what's going on.

[tivac]

@lhorie Seems pretty likely that my very simple logic does something incorrect, will fix it asap and flesh out w/ more tests once I know where you want 'em!

[lhorie]

I think the logic is fine but the issue is related to the (lack of) precision of the Date API. When it's under 16ms, it's usually somewhere around 15.8ms. Maybe that's acceptable

[tivac]

The draw console logs should be pretty accurate, assuming the environment supports the high-resolution timer it uses.

I just had a go at reproing and didn't see any immediately concerning behavior in Chrome using either requestAnimationFrame or setTimeout, but would feel better about it w/ some actual tests.

[veloce]

Hi, I'm using mithril in a big project (https://github.com/veloce/lichobile) so I'm following this closely :)

I have a suggestion for the rate limiter. Would it not be better to separate the logic of requestAnimationFrame and setTimeout, so that modern browsers would rely only on raf and would benefit from it fully (without the limitation of setTimeout)?

The frame budget of 16ms is in theory what we should attain but it's actually better to rely only on the browser to debounce input handlers, because in some cases (low battery, hidden tab, etc.) the browser may change raf callback rate.

According to the google perf guide raf can be used effectively to debounce input handlers, so the code of the rate limiter could be simplified (in the case we know raf is supported by the browser) by removing this code:

               // First render, OR if the time since the last render is greater
        // than the frame budget
        // just immediately render
        if(!last || now - last > FRAME_BUDGET) {
            last = now;

            return render()
        }

This block of code is causing problem because it renders synchronously. The google guide says visual changes should be always performed async to avoid long running input handlers. The other problem is that the rate limiter function currently mixes a synchronous rendering with an async one (the raf call at the end of the function).

Under some conditions, the browser, just after the first synchronous render() (L19) may decide to fire the raf callback right away. In that case the async render can drop under 16ms. I tested it and it happens in chrome with the test-redraw.html file, you can see it easily with a console.log() in both render blocks.

The solution to this problem, when raf is available is simply to remove the first if block that triggers the sync render. In that way all redraws will be asynchronous and driven by the browser, thus triggered at the best moment. I tested it with the same test as above and redraws don't drop below 16ms anymore.

That's why my suggestion is to separate the raf logic from the setTimeout logic to simplify things.

I don't know mithril's internal enough to know if a pure async redraw function would cause problems in some cases. In that case imo, the best option would be an api that redraws async all the time by default with the possibility to render synchronously when needed.

[dead-claudia]

@Naddiseo

Personally, I like, and use, the promise absorbing semantics of m.prop, what would replace it if it were removed?

In the 0.2 API, these two are functionally identical, mod the contagious then (which I never have a use for):

// 1.
var value = m.prop(promise)

// 2.
var value = m.prop()
promise.then(value)

That's why I'm +1 on removing the promisification of m.prop.

[dead-claudia]

@veloce IMHO all rendering by Mithril should be async. I won't have the final say on whether that will even happen, but I'm hoping it does. 😄

[veloce]

@isiahmeadows I hope too. And it's probably easier to have all rendering async by default with a synchronous one on demand than the contrary ;)

[lhorie]

The rationale for having synchronous rendering at all is related to video playing in iOS, i.e. triggering play only works if done in a context that is initiated from a user interaction, so for example, clicking a button to open a modal that autoplays a video wouldn't work w/ always-async rendering

[lhorie]

@tivac re: tests, the thing that is proving difficult to test is the size of FRAME_BUDGET, because setTimeout precision is far too coarse.

[Naddiseo]

@isiahmeadows, right, I completely forgot about using prop like that. In that case I'm +1 for simplifying m.prop further.

[tivac]

I'm not opposed to changing the rendering logic, this was a simple port of what existed in [email protected] (though I forgot to add force support, oops). Waiting to see how @lhorie wants to structure tests for all of this public API layer before I do too much more fiddling.

@lhorie I was thinking that it'd probably make sense for everything that isn't index.js/mithril.js to move out of the root into an /api folder or something, and then the tests could live in there like they do for every other chunk of functionality. Thoughts?

[lhorie]

@tivac yeah that's what I was thinking too. On a semi-related note, since you're pretty much driving this effort, do you want me to give you commit access?

[tivac]

@lhorie I'm still going to do everything via PR but sure, that'd probably help me annoy you a bit less 😆

[lhorie]

Cool, just made you a collaborator

[lhorie]

Ok thanks, let me take a look

[tivac]

Working on an API similar to the one I proposed earlier in my api-proposal branch.

Currently looking like this:

// v0.2.x style usage
var m = require("mithril");

// OR

// piecemeal usage
var m = require("mithril/api/m")
var mount = require("mithril/api/mount")
var redraw = require("mithril/api/redraw")
var prop = require("mithril/api/prop")
// etc

The default 0.2.x style API is just composed from the piecemeal components as well, so things should stay tidy-ish in that regard with a minimum of duplicated code (hopefully).

[gilbert]

For the piecemeal usage, the reason I suggested var m = require('mithril/custom') is so that other spots will only have to require that one require('mithril/custom'), as opposed to requiring everything in each file. Example:

//
// setup.js
//
var m = require('mithril/custom')

m.request = require('some-request-lib')
m.route = require('some-router-lib')
m.route.link = ...

//
// file-one.js
//
var m = require('mithril/custom')

m.request(...)
m('h1', 'hello')

//
// file-two.js
//
var m = require('mithril/custom')

m.request(...)
m('a', { oncreate: m.route.link })

This is to avoid, for example, file-two.js from having to make three different calls to require.

[lhorie]

if you're going to have a setup file, why not just copy index.js and remove the stuff you don't want?

[tivac]

I don't think aliasing third party libraries to mithril method names makes any sense at all. Just require the lib and use it.

I'm not planning on using pieces of mithril, fwiw. Mithril is small enough that ignoring the bits we don't want is more cost effective than trying to customize it.

[gilbert]

why not just copy index.js and remove the stuff you don't want?

cause then you'd have to do relative requires like require('../../lib/mithril-setup')

I don't think aliasing third party libraries to mithril method names makes any sense at all.

Sorry for the contrived example. A more realistic one would be initializing m.request with a different promise library, without the use of m.route.

And hey, given the option, I go the extra mile in cutting out kb when possible :)

[darsain]

If you don't want to split mithril into separate packages, at least make requires standardized, and comfy to use.

So require('mithril/m') instead of require(mithril/api/m). The latter is not only ugly to use, it also introduces a dependency on mithril's current directory structure, and had it ever change, I'll have to change require paths in all of my files.

So make it require('mithril/m'), where /m.js could be just a one line export referencing the real source file in /api/m.js. That way requires will be standardized and not depended on mithril's repository structure. And easier to use of course.

[tivac]

@darsain sure, this is just a POC. I don't like littering the repo root w/ random files in general so was avoiding it until there was some consensus on whether or not that experiment was worthwhile.

[darsain]

@tivac you don't have to litter the repo root with anything. Just set up a build for npm. What's published to npm doesn't have to match the repo.

[pygy]

@darsain I find it utterly confusing when I encounter such a package and try to match the require paths with the source structure in the public repo. The only way be sure of what's going on is to install the package and inspect it locally... not ideal.

[tivac]

Were the api-proposal branch to be merged I would move all the individual pieces from /api to /, in case it wasn't clear. I'm with @pygy in that I don't think doing it as a prepublish step would be very discoverable.

[tivac]

Current Status (as best I can figure) based on this issue and Leo in gitter today

• Promise Support (@lhorie has been working on this)
• m.startComputation() & m.endComputation() (if they're returning)
• shouldUpdate ➡️ onbeforeupdate
• Tests for contenteditable, custom elements, DOM recycling w/ components
• Don't print booleans (false values in views should be coerced to zero length text nodes  #1014)
• ES6 classes as components
• How to structure API for ease of partial imports ([rewrite] Rest of the API #1044 (comment), [rewrite] Rest of the API #1044 (comment), etc)

[lhorie]

Don't print booleans

that was @barneycarroll's proposal here: #1014

[gyandeeps]

We need to port changes from #1016 to rewrite for IE perf reasons. This is the left over.

CC @tivac @lhorie

[barneycarroll]

I really don't like the idea of using the return value of a mount point for granular redraws. How about redrawing components by referencing their controller / state object?

[s3ththompson]

Will there be a replacement for m.route('/myroute') as a programatic navigation method?

Also, an unrelated comment: doesn't putting route query parameters on vnode.attrs (instead of m.route.param()) make it impossible to list all query params (without including other attributes)?

[dead-claudia]

@s3ththompson To my knowledge, yes. As for the details of that, I'll refer you to @tivac and @lhorie.

[pygy]

The polymorphic m.route(...) is so far replaced by single-purpose functions.

• m.route(element, defaultPath, routeTable) doesnt' change
• m.route() => m.route.getPath()
• m.route(newPath) => m.route.setPath(newPath)
• {config: m.route} => {config: m.route.link}

[tivac]

@pygy 👍🏻

I should add that to the migration guide.

[s3ththompson]

@tivac while you're at it, perhaps add:

• m.route.param('foo') => vnode.attrs.foo
• m.route.param() => no replacement (because vnode.attrs could have other, non-query-parameter properties, right?)
• m.route.mode = 'pathname' => m.route.prefix('')

[tivac]

Opened #1090 for a place to track overall rewrite status. Discussion should continue here or in other, smaller issues on a per-topic basis.

[pygy]

Bikeshedding a bit here, but route.get and route.set would IMO be sufficiently clear and easier to remember.

[dead-claudia]

@pygy 👍 It's already obvious enough, and it would fit better with the rest of the API, which also uses more concise naming conventions.

[lhorie]

Closing this since @tivac enumerated all the actionable stuff in #1090