Mithril.js: Components don't work with ES6 classes.

Created on 12 May 2015  ·  72Comments  ·  Source: MithrilJS/mithril.js

Since ES6 classes need to be called with new and this needs to be an instance of the class, components will not work with a class. The issue stems from here: https://github.com/lhorie/mithril.js/blob/next/mithril.js#L554

Example (using babel-compiled class): https://jsfiddle.net/1prjtv78/

A work around is to wrap the component controller like:

var Component = {
   controller: function () { return new ComponentController(...arguments); }.
   view: View
};

Which transpiles to something equally ugly:

var _bind = Function.prototype.bind;
var Component = {
   controller: function () { return new (_bind.apply(ComponentController, [null].concat(arguments)))(); }.
   view: View
};

Example of working: https://jsfiddle.net/1prjtv78/1/

Perhaps the parametize.controller function can do something similar to the transpiled code.

v0.2

Most helpful comment

plz, plz make the use of classes optional. I don't want to be forced to use them. I find them cumbersome and ugly. The code is IMHO much easier to read without them.

All 72 comments

You might be interested in how I've integrated ES6 classes with Mithril components in my project: https://github.com/flarum/core/blob/master/js/lib/component.js

Usage:

class FooComponent extends Component {
  constructor(props) {
    super(props);

    this.showing = m.prop(true);
  }

  view() {
    return m('div', this.showing ? this.props.content : '');
  }
}

m.mount(document.body, FooComponent.component({content: 'bar'}));

Note that the component class itself is the controller, and the view is its method. It's a bit more like React.

That's some tricky/non-obvious code going on in component(), at first I couldn't remember what this refers to inside a static method, then I wondered "where does this.props come from?". But, interesting idea none-the-less.

An aside: I notice you don't allow parameters to the view function. I was arguing this with a co-worker today. I think a component's view should allow for arguments so that you can pass in a child vdom. For example, say you have a component that is logically a container, wouldn't you want to be able to pass in the child virtual dom to the component's view?
An overly simplistic example:

class MyContainer {
   view(ctrl, children) {
      return m('div.cool-class', {}, children);
   }
}

function View(ctrl) {
   return m('div', [
      m.component(MyContainer, [m('div', "I am a child")])
   ]);
}

m.mount(document.body, {view: View});

As for using view as a method: I'm still undecided on this in my code for two reasons:

  • Extra level of indent
  • Separation of concerns (template vs logic) in the same file and same object.

I do like being able to use this inside of the view though

TL;DR: this isn't how Mithril works. Why would you want it to work this way?

The purpose of the components structure is to provide 2 objects — a
controller constructor and a view function — such that Mithril internals
can generate and store instances of the controller according to internal
logic and pass those specific instances to the view function during redraw.
The Mithril internals that depend upon this association take care of
controller initialisation under the hood, so on the face of it whether or
not a component is a constructor or not isn't a user-land issue, except for
the fact that if the user were to generate new instances of a component
whenever they invoked it in the view, you would expect the controller
instance to be lost, what with a new object being consumed by the view —
except it isn't, because Mithril manages association internally and the old
controller instance would persist depending upon circumstances. Meanwhile,
this consistently refers to the controller instance within the controller
itself, meaning that any attempted access to this or super would be
inconsistent.

On Wednesday, 13 May 2015, Richard Eames [email protected] wrote:

That's some tricky/non-obvious code going on in component(), at first I
couldn't remember what this refers to inside a static method, then I
wondered "where does this.props come from?". But, interesting idea
none-the-less.

An aside: I notice you don't allow parameters to the view function. I was
arguing this with a co-worker today. I think a component's view should
allow for arguments so that you can pass in a child vdom. For example, say
you have a component that is logically a container, wouldn't you want to be
able to pass in the child virtual dom to the component's view?
An overly simplistic example:

class MyContainer {
view(ctrl, children) {
return m('div.cool-class', {}, children);
}
}
function View(ctrl) {
return m('div', [
m.component(MyContainer, [m('div', "I am a child")])
]);
}

m.mount(document.body, {view: View});

As for using view as a method: I'm still undecided on this in my code for
two reasons:

  • Extra level of indent
  • Separation of concerns (template vs logic) in the same file and same
    object.

I do like being able to use this inside of the view though


Reply to this email directly or view it on GitHub
https://github.com/lhorie/mithril.js/issues/618#issuecomment-101481957.

Regards,
Barney Carroll

barney.[email protected]
+44 7429 177278

barneycarroll.com

@barneycarroll, to whom are you directing that comment? If it's me: mithril should be agnostic whether or not my "controller constructor" is a plain function, ES5 class/object, or ES6 class, but it shouldn't break in the case it is an ES6 class. My problem is that mithril changes this inside the component constructor with definitely is an issue as it breaks the assumption of any constructor no matter where it's a constructor for. Nothing about using an ES6 classes (or object in general) stops mithril from functioning the way you describe.

Meanwhile, this consistently refers to the controller instance within the controller itself, meaning that any attempted access to this or super would be inconsistent.

Without using this, how do you store state? And using closures isn't really an option since they're not as performant as using objects.

_Edit:_* changing this is also an implementation detail of mithril that should not leak through to user-land code. If I pass in a class constructor, it should work since it obeys the component signature: it looks like a duck, quacks like a duck, and walks like a duck, so why doesn't mithril treat it as a duck?

@Naddiseo sorry, I just worked out what you're going on about. Bad reading on my part. The bug is that controllers don't inherit prototypes (classes are just a sugar expression that defines constructor + prototype in one).

Without using this, how do you store state?

Personally, I've never been a fan of this. The limitations, gotchas and convoluted code patterns surrounding scope application and prototypal inheritance have always been horrific. In many ways I'm disappointed ES6 gave us class because it makes things look simpler while preserving the flaky runtime. My controllers are factories. I use object literals and scope-agnostic functions wherever possible, and it makes like a lot easier (incidentally, everything is terser and you get private state):

function controller( x, y ){
  var privateX = x
  var privateY = y

  return {
    publicZ : function(){
      return privateX + privateY
    }
  }
}

And using closures isn't really an option since they're not as performant as using objects

I'd dread to think what kind of application you have that's facing measurable performance issues because of closures. You realise Mithril core doesn't use prototypes at all and the internal functions execute 1000s of times in any given view?

But I digress. For better or worse, this is expected to work with controllers. Prototypes are a core part of JS. We should fix this.

Thought this would do it, but tests are breaking. :/

@Naddiseo hey, I think this might be a false alarm caused by typos in the original fiddle (the controller's controller prototype method is never invoked) – could you check this out and see if it works for you?

Code, for reference:

let component = {
  controller : class{
    constructor(){
      this.x = 1
    }
    increment(){
      this.x ++
    }
  }, 
  view       : ctrl =>
    m('h1', 
      "Hello ", 
      ctrl.x,
      m( 'br' ),
      m( 'button', {
        onclick : e => ctrl.increment()
      },
        'Increment!' 
      )
    )
}

m.mount( document.body, {
  view: ctrl =>
    m('div', 
      component, 
      " World" 
    )
} )

@barneycarroll But now you've changed the premise by not using m.component() for the parameterization. The assessment that prototypes are a core part and this should be fixed ought still to stand. This also breaks using a TypeScript class as a controller.

Anyway, if you leave args as a parameter to parameterize() rather than slicing arguments in your changeset the tests go through and it works as expected.

plz, plz make the use of classes optional. I don't want to be forced to use them. I find them cumbersome and ugly. The code is IMHO much easier to read without them.

plz, plz make the use of classes optional

+100

The argument _for_ seems to be "I can't experience terrible flaws in the language because the library doesn't give me the opportunity". It's not a credible user story if you ask me.

+1 to @barneycarroll and @StephanHoyer.
this is a terrible feature, we're better of with lexical scoping. Simpler and more readable.

I would say that adding support for classes even as an optional feature is adding unnecessary cruft to the codebase.

But I'm also interested in hearing more from @Naddiseo and @tobscure why they want Components as classes.

@diogob, I only want to be able to use classes for the controller portion of the Component. For the view portion I do what most other people do and use plain functions and pass in a ctrl arg. As for why classes:

  1. (Opinion) I find it easier to organise my code. And, I like scoping related functions with the data they operate on.
  2. (Opinion) I prefer to store data on this than passing around multiple arguments
  3. (Sort-of-fact) When I originally benchmarked the different styles of writing the controllers, using Object.create was slow, and using lexically scope function was comparable but slower to using classes It seems those benchmarks may no longer hold up, I'll create some other tests and see if my assumptions still hold water.

Whatever the case may be, my point earlier in the thread still holds: if mithril uses new on the controller, it should be unaware of where it's a function or a class, thus classes would still be optional. What this issue is about is that mithril breaks the assumptions I can make about the controller; mithril tells us that it is newed, so I would expect passing anything new-able would work.

if you just want that class as your controller why not just pass it like this controller: {controller: yourClass, view: yourView, then i think it will work as you expect.

ops, it will not work. sorry..

@syaiful6 @Naddiseo I realise this is a huge point of irritation when trying to rationalise object schemas and instances. I've recently proposed a new syntax for Mithril components that might interest you in 499.

One great thing about classes is that you have full code-completion, you dont have to remember any names just select them.

@daslicht BTW, that point is moot if you consider that current components are almost classes conceptually.

IMHO, I see other reasons to integrate classes and/or functions, though. A great example is developer ergonomics and debugging at scale - think of the React Developer Tools extension for Chrome. It's pretty helpful for React developers. Without the ability to get the name of a component, it's not possible to create that kind of thing for Mithril. And if components are plain objects, that's not possible (objects don't have a name property like ES6 functions and classes do, nor do they have a displayName like functions in IE). Oh, and if a component returns the wrong thing (e.g. forgetting to return a node from your view, an easy and common mistake), you can at least use the name of the class to make the error message more descriptive, since stack traces won't help. You don't get that luxury with object literals.

@lhorie You might appreciate the above paragraph for your rewrite.

I don't care how it's called unless you get full code-completion.
That works currently even across files (at least with TypeScript)!
So that you don't have to remember, or look up how things are named.
Just write a Class _one_ time and forget the naming of its members.

Just choose from a list without even tying. That makes meaningful names much faster to use than typing them manually 1000 times.
If that workflow could be offered by functions I would use functions.

@daslicht Can you choose from a list of everything in scope that satisfies a type? I don't use a lot of TypeScript or IDEs of any kind.

@daslicht typing the same thing out 1000 times sounds like a lot of hard work. Can you point to any example of a meaningful name in you have typed out more than 10 times in the context of a Mithril component?

My contention is that "classes are awesome" is a truism that can't be related to this topic in a practical way. None of the advocates have mentioned any scenario where this might be of use except in the most general way possible: the onus is on people who've actually looked into the code and use it without classes to reimplement the API with classes so that people who like classes can see whether or not it's useful to them.

It doesn't sound like a very convincing or appealing exercise.

@barneycarroll I did mention developer ergonomics - it's not possible to develop things like React Dev Tools or similar for Mithril, since object components don't have that magic name property functions and classes do. That kind of thing helps with larger web apps, if you're troubleshooting something with UI content that's off. (No, unit testing doesn't help you fix logic. It only tells you if the logic is broken.)

That might not appeal to you, since IIRC you don't use it much at scale, but I know some do, and that ability would be incredibly powerful for those use cases.

It also helps the more kinesthetic, tactile programmers as well, which tend to do best with a very powerful, detailed autocomplete and GUI graphs of all the nodes in the tree, moving in real time, like in Light Tables. IIUC, you're best with just text in that tab completion with identifiers and some mild autocomplete works perfectly fine for you most of the time, and pictures usually just get in the way of your usual routine. I'm more of a mixture, where I need a little of both. It really comes down to how people think (and how no two think alike).

So I understand how it wouldn't appeal to everyone, but since people think best in different ways (classes are a little more visually descriptive, while objects are a little more concise and easier to textually digest), and both versions have their merits (components are easier to generate from factories if they're objects, while classes are easier to statically analyze), that's why I've always supported both versions from the very start.

I'll fill in the blanks using my imagination: if you were using Typescript with an IDE you would type m( and get a dropdown of all available components in your codebase. Plugging in the Mithril inspector would let you visually map components and their internal state at run time. Is this what we're after?

@barneycarroll The former you should already be able to do _now_ with most TypeScript IDEs (like Atom TypeScript or VS). The latter is mostly an optional thing. When I'm writing React code, I rarely consult the React Dev Tools, but I've seen a lot that really love it. As for the other side of the deal, some people conceptualize far better with classes instead. I've met several people that way.

Oh, and at some point, when you're writing at scale, and this is the key part you seem to be missing, there's a reason why Flarum wrote a wrapper for class-based components. They wanted a consistent structure for components, and this is usually where classes tend to excel. I do agree that there are a few things that could be simplified, but consistency and clear structure helps when you're writing a large app.

Would you prefer a slightly lower barrier of entry and increased scalability for those writing Mithril, and more people adopting and liking the framework, or do you want it to become very opinionated in favor of free-form components, in a non-mainstream way, potentially alienating existing users as well as making life difficult for those writing large Mithril apps?


And in direct reply to your comment:

I'll fill in the blanks using my imagination: if you were using Typescript with an IDE you would type m( and get a dropdown of all available components in your codebase. Plugging in the Mithril inspector would let you visually map components and their internal state at run time. Is this what we're after?

Only if you want it. I'm promoting enabling the option, not making that the default workflow. Neither of those will have native integration into core, and they never should. For what it's worth, even mithril-objectify isn't in core. Just because something exists doesn't mean you have to use it.

alienating existing users as well as making life difficult for those writing large Mithril apps?

I'm lost. What's the alienating difficulty we're proposing? There seems to be a subtext that I develop Mithril applications that aren't large enough — people who like classes are presumably building larger and more important things. I don't really know how this inference was made or how it relates to the subject under discussion. I was going to say "specifics under discussion", but we seem to be drifting further and further away from any chance of that.

You're absolutely correct in saying Mithril Objectify isn't core. None of the Mithril plugins I developed to support my app are core, either. Again, I don't know how to tie this back to the perceived problem or what a solution might look like.

Let's take a step back. I came into this thread to try and work out what the petitioners are asking for in practical terms, and wondering what work could be done to alleviate those issues — specifically, my contention is that "we want classes" as a movement doesn't seem to have any clear developer user stories that we might use as qualifications of success in trying to develop a solution to this issue.

I'm promoting enabling the option

Can you go into more detail about this?

I'm currently using ES6 classes as components, mainly for separation of concerns and ease of debugging for complex situations (D3 visualizations or a dynamic HABTM menu for example)

Here is a basic example for this: codepen

If mithril were to change to detecting prototype in component or whatever, and then constructing the Class I think this fundamental change to the API would make it harder to reason about using classes. Currently this is easily handled via class static methods, and I would prefer to keep explicitly constructing my classes in a uniform way.

However, if you feel the feature is worth pursuing it might be prudent to detect if the class accounts for Mithril wanting controller and view properties before constructing the instance?

Logically, I find it much more transparent to think of the instance as the current controller's internal state, than the component's instance, and the view should be static relative to the controller instance.

I'm not a zealot, I just though I'd weigh in on how i'm currently using classes myself, and a few others I've converted are using mithril with es6.

Related comments by me on other ways to do components in Mithril:
https://github.com/lhorie/mithril.js/issues/733#issuecomment-201945393
https://github.com/lhorie/mithril.js/issues/499#issuecomment-201949762

@ondreian

Actually, it would be a simple typeof component === "function" check to see if it's a class (arrow functions would fail to construct, and would not initially be valid components). That simple. Pure components will complicate the picture, but it's undecided if they're even necessary, considering the following helper (I personally use a variant of this now):

// function pure(func: (...args) => VirtualNode)): Component<any>
const pure = func => ({view: (_, ...args) => func(...args)})

const TagList = pure((post, isTag, resolvedTag) => m(".post-tags", [
    m("span", "Tags:"),
    post.tags
    .map(tag => [tag, isTag && tag === resolvedTag ? ".post-tag-active" : ""])
    .map(([tag, active]) => m(`a.post-tag${active}`, route(`/tags/${tag}`), tag)),
]))

(Yes, I'm more of a functional programmer.)

_Edit: fix bug in plugin_

And in a conversation I had with @barneycarroll in Gitter privately, one thing I came up with for how ES6 components could feasibly be supported now: use this wrapper function.

function classToComponent(C) {
    return {
        controller: function () { return new C(...arguments) },
        view: (ctrl, ...args) => ctrl.view(...args),
    }
}

If you want to try this now, you might be able to use this Mithril wrapper plugin (it's in CoffeeScript for algorithmic clarity, but it's easily translated into JS):

classPatch = do ->
  'use strict'

  classToComponent = (Component) ->
    controller: (args...) -> new Component(args...)
    view: (component, args...) -> component.view(args...)

  coerce = (func, self) -> (ctrl, args...) ->
    if typeof ctrl == 'function'
      func.apply self, [classToComponent(ctrl)].concat(args)
    else
      func.apply self, arguments

  classPatch = (old) ->
    m = coerce(old, undefined)
    # If you're unfamiliar with CoffeeScript, this is their `for ... in` loop
    for own prop of old
      m[prop] = old[prop]
    m.component = coerce(old.component, old)
    m

# Usage:
m = classPatch(m)

@barneycarroll Regarding Classes ind Mithril you might be rirgth. I dont use Mithril yet so I cant tell much about it and dont have any example. It is ages ago i worked wuth real classes, last Time it was as I coded with Adobe Flex years ago. There we had full refactoring of Names across the whole project, even imports.You could move files and rename and anything stayed valid. We created value objects which represents for example a user andautocomplete allowed us to just choose values. So you had to implement it once and were abe to use it in as many new projects you like and never had to type any of its properties by hand. That is just one example, probably that makes it more clear what I meant by tyimg 1000 times.

@isiahmeadows Yes autocomplete just shows the public members and not all globals.

@daslicht There's a few people, though, that would love to see classes make it into core, though. One big example would be with Flarum, where they themselves monkey-patched Mithril to work with almost React-like components (they use JSX + a couple internal Mithril plugins), and my proposed change would probably mostly only affect these two files (if they choose to take advantage of this), if I'm reading their code base correctly.

@isiahmeadows Flarum dev here. Having classes work out of the box would be nice, although our implementation differs slightly from the patch that you've provided, in that we do some setup to store args on the component instance (so we can have this.props like in React). I don't think that's necessarily a good paradigm though... certainly if Mithril came with built-in support for classes, we'd look at adapting our components to fit in with the way it does things so we can run without a patch.

We've got some changes in the pipeline regardless (extracting our component paradigm into an external package), so now would be a great time for this to happen :D

@tobscure

  1. You would still be able to store arguments on the instance and use that specifically:

``` js
class Component {
constructor(props, ...children) {
this.props = props
this.children = flatten(children)
}

 view() {
   return m(".whatever", [
     "Do something with ", this.props, " and ", this.children,
   ])
 }

}
```

  1. I did find that it wouldn't necessarily mean removing all the boilerplate. You still might need to patch it some, but you wouldn't need to patch m(). You'll still need some non-trivial boilerplate in your Component abstract class, though, given how you've architected the components.

The way I would be implementing this would be by effectively duplicating the functionality of this function to work with classes.

Actually...I just realized a hitch...I need to also wrap the components on m.mount and friends as well... :(

This just got complicated.

@isiahmeadows I like your classToComponent wrapper function. As a reminder, this can also be done with static methods directly on the class. I do that with TypeScript but I would think it should work the same in ES6.

 class StoryBrowser {
    ...
    static controller(args) {
        return new StoryBrowser(args);
    }

    static view(controller, args) {
        return controller.calculateView(args);
    }
    ...

@pdfernhout I'm aware. I'm working on implementing it in core, though. Somehow, it's actually resulting in simpler code apart from this dispatcher function (trying to avoid an expensive Function.bind call):

function constructWithArgs(C, args) {
    // Try to fast-path with a direct call first.
    switch (args.length) {
    case 0: return new C()
    case 1: return new C(args[0])
    case 2: return new C(args[0], args[1])
    case 3: return new C(args[0], args[1], args[2])
    case 4: return new C(args[0], args[1], args[2], args[3])
    default: return new (C.bind.apply(C, [undefined].concat(args)))()
    }
}

The gitter discussion is moving too fast right now...

To differenciate reliably between functions and classes, you may want to provide a base class with a sentinel and check for its presence as an inherited property...

function Component(){}
Component.prototype.sentinel = {}

if (Foo.x ===Component.prototype.sentinel) { return new Foo() }
class MyComponent extends m.Component {}

m(MyComponent)

AFAICT it should work with both native and transpiled classes (either Babel or Bublé)

Edit... or simply using instanceof.

Edit2: I now see @isiahmeadows suggested something similar in the chat:

@lhorie Here's how React checks pure components: it checks to see if the function has a prototype and is a React.Component subclass.
That is much simpler to do, but it's not trivial. Another idea is to check to see if the function has a view instance method, which is required for components.

Edit 3: too fast, not too fart :-/

@pygy I was hoping for something that doesn't require a base class, but in the absence of such a solution, that's actually a pretty elegant alternative

@lhorie @pygy

Edit: corrected a couple things

One of the things in the Gitter discussion I mentioned (I think...it was moving really fast) was this:

function isPureComponent(C) {
    return C.prototype != null && typeof C.prototype.view === "function"
}

I also suggested, for the rewrite, you can implement a layer of abstraction like below. You would, instead of working on the component directly, operate on its respective wrapper by using getWrapper(vnode.tag). Otherwise, it's practically a drop-in replacement. This could go on the component itself as well.

// I'm using ES6, but it's trivial to translate to ES5.
const wrapperMap = new Map()

function getWrapper(component) {
    if (wrapperMap.has(component)) {
        return wrapperMap.get(component)
    } else if (typeof component === "function") {
        return new FunctionWrapper(component)
    } else {
        return new ObjectWrapper(component)
    }
}

class ObjectWrapper {
    constructor(component) {
        this.component = component
        wrapperMap.set(component, this)
    }

    oninit(vnode) {
        vnode.state = {}
        this.component.oninit(vnode)
    }

    oncreate(vnode) { this.component.oncreate(vnode) }
    onupdate(vnode) { this.component.onupdate(vnode) }
    onbeforeremove(vnode) { this.component.onbeforeremove(vnode) }
    onremove(vnode) { this.component.onremove(vnode) }
}

class FunctionWrapper {
    constructor(C) {
        this.component = component
        this.pure = C.prototype != null && C.prototype.view != null
        this.tree = null
        wrapperMap.set(component, this)
    }

    oninit(vnode) {
        if (this.pure) {
            vnode.state = {}
            this.tree = (0, this.component)(vnode)
        } else {
            vnode.state = new this.component(vnode)
        }
    }

    oncreate(vnode) { if (!this.pure) vnode.state.oncreate(vnode) }
    onupdate(vnode) { if (!this.pure) vnode.state.onupdate(vnode) }

    onbeforeremove(vnode, done) {
        if (this.pure) return done()
        vnode.state.onbeforeremove(vnode, done)
    }

    onremove(vnode) { if (!this.pure) vnode.state.onremove(vnode) }
}

In ES5, the map could be implemented with a pair of arrays and an LRU cache (I'd tier it a few times to help larger apps), but this should provide the gist. (@lhorie, apologies if I wasn't clear enough :wink:)

I propose the component passed to Mithrill must be a callable (pure function). This function just a factory, i call this component factory.

Component factory is a pure function, and should return an object wheter is plain object or instance of class with same signature as current Mithrill. Mithrill should call this function normally without new keyword. If the function not return what we expect like there are no view attributes, Mithrill can complain loudly (ie throw Error). Or if in the way Mithrill call this function an error occured let it be, so the developer know how to deal with it.

That way it help the library simplify the API, and not require user to extends base class or bothering whether the component is pure function or classes.

We dont need to know the function passed to us are pure function or not, just call it, if there are no error occured then check the object signature returned. If errror occured let it be, ie Babel transpilled code throwing Error.

Example taken from Mithrill

Current API:

var MyComponent = {
    controller: function(data) {
        return {greeting: "Hello"}
    },
    view: function(ctrl) {
        return m("h1", ctrl.greeting)
    }
}

I propose:

function my_component() {
    return {
        controller: function(data) {
            return {greeting: "Hello"}
        },
        view: function(ctrl) {
            return m("h1", ctrl.greeting)
        }
    };
}

That way, people with like to write them as classes can do something like:

function my_component() {
    return new MyAwesomeClass();
}

For people using classes, maybe write function just to instantiate their class are really too annoying. To solve this issue Mithrill can provide a function decorator for them, something like:

function decorateClass(cls, ...args) {
    return new cls(...args);
} 

Deprecation:

For people that write a component based plain object like the sample code on Mithrill home, we can detect this. So we can wrap them to a pure function for purpose of this new API. Mithrill can log the message to console so we know it going to removed in the future.

@syaiful6 My proposal for the rewrite can be very easily refactored to permit that as well.

(By the way, most of the design work is going into the rewrite, since it's already here. Feel free to check it out, but do be aware it's far from production-ready.)

 class FunctionWrapper {
     constructor(C) {
         this.component = component
         this.pure = C.prototype != null && C.prototype.view
+        this.stateful = !this.pure
         this.tree = null
         wrapperMap.set(component, this)
     }

     oninit(vnode) {
         if (this.pure) {
             vnode.state = {}
-            this.tree = (0, this.component)(vnode)
+            const ret = (0, this.component)(vnode)
+            if (this.stateful = (ret.view != null)) {
+                vnode.state = ret
+            } else {
+                this.tree = ret
+            }
         } else {
             vnode.state = new this.component(vnode)
         }
     }

-    oncreate(vnode) { if (!this.pure) vnode.state.oncreate(vnode) }
-    onupdate(vnode) { if (!this.pure) vnode.state.onupdate(vnode) }
+    oncreate(vnode) { if (this.stateful) vnode.state.oncreate(vnode) }
+    onupdate(vnode) { if (this.stateful) vnode.state.onupdate(vnode) }

     onbeforeremove(vnode, done) {
-        if (this.pure) return done()
+        if (!this.stateful) return done()
         vnode.state.onbeforeremove(vnode, done)
     }

-    onremove(vnode) { if (!this.pure) vnode.state.onremove(vnode) }
+    onremove(vnode) { if (this.stateful) vnode.state.onremove(vnode) }
 }

That's great.. i dunno it already rewrite, hmmm, i will take a look. of course i dont use them on production!

There are a library implements lru cache, but we can write ourself for this purpose. thanks..

I will note that the rewrite is backwards-incompatible.

@lhorie was working on it for a while, and once it got here, of course a lot of us were eager to look at it.

For rewrite, I'd like the design for this proposal to be top-down first, and be informed by implementation details second.

In my mind, if ES6 classes are to be supported, the ideal syntax should be:

//definition
class Foo {
  view() {return "hello"}
}

//consumption
m(Foo)

As for implementation: ideally, this should also be possible

//definition
function Foo {
  return "hello"
}

//consumption
m(Foo)

The issue is that a) both classes and functions have the same type and it's generally not possible to distinguish between them without calling them or peeking at its toString(), b) they have different calling restrictions (classes must be called w/ new and arrows must not be called without new, and c) calling a primitive-returning function w/ new loses the return value

The idea of using a base class is the closest to the ideal so far. Ideally, we need a non-hacky isClass implementation of some sort that passes these tests

For the record, from some quick benchmarking, peeking at toString appears to be 2 order of magnitude slower than calling the function

What do you look for in the toString() output? I'm afraid you can detect a class compiled by Bublé by looking at the source, where

class Foo{
  view () {
    return "hello"
  }
}

becomes

var Foo = function Foo () {};

Foo.prototype.view = function view () {
  return "hello"
};
"function Circle( radius ) {
    Shape.call(this);
    this.radius = this;
  }"

toString can tell you definitively that the thing is a ES6 class or an arrow, though it doesn't help with other edge cases

Please don't even consider checking classes with toString. So slow. Wouldn't this be enough?

let result;
try {
  result = fn(); // fails when class
} catch (e) {
  result = new fn();
}
let rendered = result && result.view ? result.view() : result;

Adding my 0.02c:

Enforcing components to extend from a base class seems too opinionated for mithril, so I would be -1 for that solution. Without another alternate, I think @syaiful6's solution is better since it's allowing us the flexibility to decide how to structure the code by giving us an interface (a function that returns something with a view function).

As an aside, I definitely prefer components that are plain JS objects. I think I've missed some of the discussion here (I don't follow gitter), why do components have to be callables?

They don't _have_ to be, but if it's low complexity, I don't mind adding support for class-based components, since some people like them.

Pure function components are nice-to-have because they require less typing and can be composed by FP-oriented folks

Class-based components are nice-to-have for the more OOP-oriented folks.

One use case that is currently not very nice is component methods. Right now they have to be procedurally attached to vnode.state. I was thinking of pointing this in lifecycle methods to vnode.state to make that more tractable, but declaring methods in the component object declaratively a la React.createClass is currently not supported. It would be nice if it was, and if we could support classes with the same code, it would be even better.

@lhorie I mentioned in Gitter that you could wrap the hyperscript API to work with the wrappers in my comment pretty easily. This wouldn't even theoretically need to be in core, but it could be better optimized if it were in core (i.e. consulted less).

const m = (old => (type, ...args) => {
    if (typeof type === "object" && type !== null || typeof type === "function") {
        return old(getWrapper(type), ...args)
    } else {
        return old(type, ...args)
    }
})(require("mithril/render/hyperscript"))

For rewrite, I'd like the design for this proposal to be top-down first, and be informed by implementation details second.

Also, for the above reason (able to be completely independent of core), I don't think it is that big of a deal regarding implementation details. It's enough of a drop-in replacement that I doubt it'll be much of a problem.

@isiahmeadows I think implementing a LRU cache qualifies as too complex. Also I'm not entirely sure how your code is meant to work

You suggested:

const m = (old => (type, ...args) => {
    if (typeof type === "object" && type !== null || typeof type === "function") {
        return old(getWrapper(type), ...args)
    } else {
        return old(type, ...args)
    }
})(require("mithril/render/hyperscript"))

so presumably, getWrapper returns a component if type is a object or function.

In the other snippet, you have this:

// I'm using ES6, but it's trivial to translate to ES5.
const wrapperMap = new Map()

function getWrapper(component) {
    if (wrapperMap.has(component)) {
        return wrapperMap.get(component)
    } else if (typeof component === "function") {
        return new FunctionWrapper(component)
    } else {
        return new ObjectWrapper(component)
    }
}

There, it says getWrapper returns an ObjectWrapper instance if the component is an object. The code already handles objects, so I'm not sure what's the point of doing this

class ObjectWrapper {
    constructor(component) {
        this.component = component
        wrapperMap.set(component, this)
    }

    oninit(vnode) {
        vnode.state = {}
        this.component.oninit(vnode)
    }

    oncreate(vnode) { this.component.oncreate(vnode) }
    onupdate(vnode) { this.component.onupdate(vnode) }
    onbeforeremove(vnode) { this.component.onbeforeremove(vnode) }
    onremove(vnode) { this.component.onremove(vnode) }
}

The ObjectWrapper class has no view method, so I'm not sure how that renders. It sounds like it would throw here: https://github.com/lhorie/mithril.js/blob/rewrite/render/render.js#L91

Also, I'm not sure what all the event delegation stuff is for.

class FunctionWrapper {
    constructor(C) {
        this.component = component
        this.pure = C.prototype != null && C.prototype.view != null
        this.tree = null
        wrapperMap.set(component, this)
    }

    oninit(vnode) {
        if (this.pure) {
            vnode.state = {}
            this.tree = (0, this.component)(vnode)
        } else {
            vnode.state = new this.component(vnode)
        }
    }

    oncreate(vnode) { if (!this.pure) vnode.state.oncreate(vnode) }
    onupdate(vnode) { if (!this.pure) vnode.state.onupdate(vnode) }

    onbeforeremove(vnode, done) {
        if (this.pure) return done()
        vnode.state.onbeforeremove(vnode, done)
    }

    onremove(vnode) { if (!this.pure) vnode.state.onremove(vnode) }
}

Same here, no view method, so how does this render?

The third line is a null ref (I'm guessing you meant s/(C)/(component)/?).

The line after that yields different boolean values for function() {this.view = function() {}} and class {view() {}}. I'm guessing it shouldn't

The oninit looks like it would throw if component is a arrow function, because then this.pure == false which causes oninit to go into the new component conditional branch

this.tree is never used

Generally, I just don't understand what this code is supposed to be doing

@lhorie

  1. The missing methods should be checked for, but I initially omitted the checks for brevity.
  2. You're correct in that component within the FunctionWrapper constructor should've been C. Also, this.pure should've been defined as the opposite: C.prototype == null || C.prototype.view == null. That was a dumb mistake on my part.
  3. > The line after that yields different boolean values for function() {this.view = function() {}} and class {view() {}}. I'm guessing it shouldn't

It should. You'll need to add a view _prototype_ method to tell Mithril the class satisfies the interface requirement, even if it's just as simple as C.prototype.view = function () {} (if you're using closure-local variables). The method is still called on the instance itself, so it shouldn't be a problem. There's no other way to tell otherwise.

  1. I forgot the view and shouldUpdate methods. Also, I screwed up several other methods in the process. See the snippet below for what I _should_ have posted. (I didn't look over my snippet as much as I thought I had...)

Here's the fixed snippet (minus the suggestion to allow pure functions to return a component, which isn't very different from just using classes). Notes:

  • This uses closures because currently, the methods are currently called with a specific this. It's also a little more clear and concise this way. I did retain this invariant with objects.
  • It verifies the method exists first before actually calling it. The only exception is with view.
  • It always delegates the method calls for class instances. If you assign a view in the constructor, that one is used instead.
  • Pure components have no lifecycle hooks, and the state property is intentionally undefined, because pure components should be completely stateless, anyways.
  • This doesn't actually check the types, but assumes everything is correct. If it's not, it'll fail, anyways.
// I'm using ES6, but it's trivial to translate to ES5, mod the WeakMap.
const wrapperMap = new WeakMap()
const hasOwn = {}.hasOwnProperty

function getWrapper(C) {
    if (wrapperMap.has(C)) {
        return wrapperMap.get(C)
    }

    const wrapper = wrapComponent(C)
    wrapperMap.set(C, wrapper)
    return wrapper
}

function isPure() {
    return C.prototype == null || C.prototype.view == null
}

function wrapComponent(C) {
    if (typeof C !== "function") return wrapObject(C)
    if (isPure(C)) return wrapPureFunction(C)
    return wrapClass(C)
}

function wrapStateful(_) {
    function call(name, vnode) {
        return _.has(vnode, name)
            ? _.call(vnode, name)
            : undefined
    }

    return {
        oninit(vnode) { return _.oninit(vnode, call) },
        oncreate(vnode) { return call("oncreate", vnode) },
        onupdate(vnode) { return call("onupdate", vnode) },
        onremove(vnode) { return call("onremove", vnode) },

        shouldUpdate(vnode) {
            return !_.has(vnode, "shouldUpdate") ||
                _.call(vnode, "shouldUpdate")
        },

        onbeforeremove(vnode, done) {
            return _.has(vnode, "onbeforeremove")
                ? _.call(vnode, "onbeforeremove", done)
                : done()
        },

        view(vnode) { return _.call(vnode, "view") },
    }
}

function wrapObject(C) {
    return wrapStateful({
        oninit(vnode, call) {
            vnode.state = {}
            return call("oninit", vnode)
        },

        has(_, key) { return hasOwn.call(C, key) },
        call(vnode, key, ...rest) { return C[key].call(vnode, vnode, ...rest) },
    })
}

function wrapClass(C) {
    return wrapStateful({
        oninit(vnode) { vnode.state = new C(vnode) },
        has(vnode, key) { return hasOwn.call(vnode.state, key) },
        call(vnode, key, ...rest) { return vnode.state[key](vnode, ...rest) },
    })
}

function wrapPureFunction(C) {
    return {
        oninit(vnode) { vnode.state = undefined },
        oncreate(vnode) { return undefined },
        onupdate(vnode) { return undefined },
        onremove(vnode) { return undefined },
        shouldUpdate(vnode) { return true }, // Always check the tree
        onbeforeremove(_, done) { return done() },
        view(vnode) { return C(vnode) },
    }
}

Does this help?

@isiahmeadows not really. It still looks like a lot of code especially considering WeakMap requires a non-naive polyfill

And again, C.prototype.view == null is true in a crockford-style class

function Component() {
  this.view = function() {}
}

console.log(Component.prototype.view) // undefined

So that ultimately ends up returning an invalid vnode in wrapPureFunction's view method

@lhorie I'll try to explain it in prose + visual aids. I think the problem is the medium I'm explaining it in.


Here's a brief description of how I see it could be handled.

When initializing a component:

  1. If it's an object, set the vnode's state to an empty object, and call oninit with the vnode, like in component.oninit.call(vnode, vnode).
  2. If it's a constructible function with a view prototype method, call it as a constructor with the vnode, and set the vnode's state to the result. This vnode is considered stateful.
  3. If it's a function either non-constructible or without a view prototype method, call the function with the vnode.

    1. If this returns an object with a view method, then set the vnode's state to the result. This vnode is considered stateful.

    2. Otherwise, remember the resulting tree for this component. This vnode is considered stateless, and the vnode's state is undefined.

Here's an ASCII graphic to help you out, in case you're more visually oriented:

      object? -> `vnode.state = {}` -> `component.oninit.call(vnode, vnode)`
         |
        \|/
 component is ... -> constructor with `view` method? -> `new Component(vnode)`
         |                                                       |
        \|/                                                      |
function without `view`?                                         |
         |                                                       |
        \|/                                                     \|/
 `component(vnode)` ----------> has `view`? ---------> `vnode.state = result`,
         |                                                vnode is stateful
        \|/
     no `view`?
         |
         |------> remember resulting tree -> `vnode.state = undefined`,
                                                 vnode is stateless

When calling a hook:

  1. If no such hook exists, do nothing.
  2. If the vnode's component is an object, call the hook with the vnode, like in component.onremove.call(vnode, vnode).
  3. If the vnode's component is a function:

    1. If the vnode is stateful, call the hook as a method of the vnode's state with the vnode, like in vnode.state.onremove(vnode).

    2. If the vnode is stateless, return true for the shouldUpdate hook, but do nothing otherwise.

Here's an ASCII graphic for this part:

hook exists? -> no? -> do nothing
     |
    \|/
    yes? -> component is ... -> object? -> `component[hook].call(vnode, vnode)`
                  |
                 \|/
              function?
                  |
                 \|/
          vnode is stateful? -> yes? -> `vnode.state[hook](vnode)`
                  |
                 \|/
                 no? -> hook is `shouldUpdate`? -> `true`
                  |
                  |---> hook is anything else? -> do nothing.

When calling the view:

  1. Assert: the vnode has the required view, and it is a function.
  2. If the vnode's component is an object, call the view with the vnode, like in component.view.call(vnode, vnode).
  3. If the vnode's component is a function:

    1. If the vnode is stateful, call the view as a method of the vnode's state, like in vnode.state.view(vnode).

    2. If the vnode is stateless and a tree is remembered, return the tree and forget it.

    3. Otherwise, call the vnode's component as a function with the vnode, like in component(vnode)

Here's an ASCII graphic for this part:

component is ... -> object? -> `component.view.call(vnode, vnode)`
      |
     \|/
   function? -> vnode is stateful? -> yes? -> `vnode.state.view(vnode)`
                       |
                      \|/
                      no?
                       |
                      \|/
                tree remembered? -> yes? -> return and forget tree
                       |
                       |----------> no? -> `component(vnode)`

I think it conceptually is too large of a concept to easily digest in code or text.

I want to share my two cents here, our team has been use mithril with coffee/es6 class for a long time(though i prefer coffee for simplicity), you can check out my own (ui collection)[http://winterland1989.github.io/mui/] to get a big picture, basically you can use following pattern to make ui component:

class BiggerComp
  constructor: ->
    @childComp = new ChildComp
      ...

  eventHandler: (e) =>
    ...

  view: ->
    m '.BiggerComp',
      ...
        onEvent: @eventHandler
      ...
      @childComp.view()

I think this's where mithril.js really shines: it doesn't force you into certain pattern to structure your program. you can use class to inject your model, or use plain constructor functions like controller. following code is absolutely ok and performant:

m = require 'mithril'

class Demo
    constructor: ->
        ...

    view: -> [
        ...
    ]

m.mount main, new Demo()

@winterland1989

Most of the discussion at this point is whether to natively support classes themselves (not merely instances of them) as components in core, side-by-side with objects. It'd be closer to m.mount main, Demo what I've been suggesting.

Most of the discussion at this point is whether to natively support classes themselves (not merely instances of them) as components in core, side-by-side with objects. It'd be closer to m.mount main, Demo what I've been suggesting.

May i ask why? As far as i can see, you are suggesting we should instantialize class automatically inside m.mount, but how do you inject initial model/state without change m.mount's signature?

@winterland1989 via vnode.attrs and vnode.children, e.g. m(MyComponent, {foo: "bar"})

i see, but this will change m's semantic, since m is supposed to be evaluated eagerly without cache(without subtree retain optimization), if we instantialized a stateful component using m, there must be some magic should be cast, no?

Working on this now for the rewrite.

Adding a few ¢ to this thread.

For stateless components, using a POJO is totally fine; I don't see much reason to use classes. So this will mostly be about stateful components.

For stateful components that have their own properties or methods, I think POJOs are slightly confusing when you first start using them, as they are used by Mithril as a template to create instances. What this means in a component method took some getting accustomed to. Using a class as a component may be a bit more intutive to beginners (at least for those who come from class based languages.)

For the rest of this comment I'm going to talk about Typescript, which I realize must represent a tiny minority of Mithril users, so I'm not sure how much effort should be spent accomodating us. Typed JS variants do seem to be gaining popularity however and I think these create some additional use cases for classes.

First, it's a little more conventional to declare a class to take advantage of types than to do something like this. Not that it's a terrible solution but it does have drawbacks.

Now that Typescript supports the rather nice feature of non-nullable types (as Flow does) it can be awkward to declare a non-nullable property on a typed POJO if you can't assign it until oninit runs. Example:

interface MyComponent extends Mithril.Component {
    foo: Foo
}

const myComponent: MyComponent = {
    foo: undefined, // <- can't do if we want non-nullable, can't omit either.
    oninit (this: MyComponent, vnode: Mithril.Vnode) {
        this.foo = new Foo(vnode.attrs.fooInitializer)
    },
    view() {/*...*/}
}

In the above case, I want to treat foo as non-nullable because I know it will always have a value, but Typescript can't really know that oninit gets called first (sort of like a constructor.)

Now interestingly, neither Typescript or Flow can actually detect this problem:

class MyComponent implements Mithril.Component {
    foo: Foo // this.foo is undefined but no compiler error
    constructor() {}
    oninit (vnode: Mithril.Vnode) {
        this.foo = new Foo(vnode.attrs.fooInitializer)
    }
    view () { ... }
}

So that would compile happily. Simply allowing classes to be used as components gets around the problem of having non-nullable property assignment deferred until oninit. For now anyway. There is the desire to fix that problem in future, so this workaround may not work forever.

What might be a better, long-term solution is to have the constructor called instead of/in addition to oninit and be provided with the Vnode.

class MyComponent implements Mithril.Component {
    foo: Foo
    constructor (vnode: Mithril.Vnode) {
        this.foo = new Foo(vnode.attrs.fooInitializer)
    }
    view () { ... }
}

For reference, here are types for mithril 1.0 that I've been hacking away on.

https://github.com/spacejack/mithril.d.ts

Folks, I'd love to have your feedback on #1339

You can play with it here: https://github.com/j2css/mithril.js/blob/b581e5a1b4843eca3937835b62714b160dcb6b8c/mithril.js

For classes:

class MyComponent extends m.Component {
  constructor(vnode) {
    // initialize things here
    // vnode.state doesn't exist yet, but you can access its future incarnation as `this`
  }
  // define hooks here if you want
  oncreate() {}
  view() { return "Classy" }
}
m.render(document.body, m(MyComponent))

For factories:

function MyFactory(vnode) {
  // initialize things here (don't rely on `vnode.state` yet)
  var state = {
  // define hooks here if you want
    oncreate() {},
    view(vnode) {
      // this === state === vnode.state
      return "Industrial"
    }
  }
  return state
}
m.render(document.body, m(MyFactory))

In both cases oninit is redundant but preserved because it would take more code to avoid calling it for non-POJO components. It is always optional.

@pygy that factory example is very interesting. It would be very nice to have a native way to write mithril components that don't need this or vnode.state.

I always imagined the various hooks to be passed in to the factory as streams, and for the factory to return a view stream and nothing else. But I think I like this better ( though I like hooks as streams as an idea ( I'd like if the router used streams too) ).

It would be nice to be able to return either a state object or simply just a view. Most components I'll write won't use any hooks, so wrapping the response seems a shame.

So this: would be a valid component

function MyFactory(vnode) {
  return () => "nice pun!"
}

m.render(document.body, m(MyFactory))

And because props are functions, you could return a view stream

function Adder(vnode){
  const a = m.prop(0)
  const b = m.prop(0)

  const sum = m.prop.combine( () => a() + b(), a,b)

  const view = sum.map(function(){
    return [
       m('input[type=number]', { oninput: m.withAttr('value', a) })
       ,m('input[type=number]', { oninput: m.withAttr('value', b) })
       ,m('Sum: ', sum() )
    ]
  })

  sum(0)

  return view
}

m.render(document.body, m(Adder))

I'd really like to be able to just return a function optionally, I think it will be the common case and could open up some interesting optimizations down the line if the view is a stream.

But I could happily live with the current proposal of just returning state too.

Here's a typescript example using a class component:

https://github.com/spacejack/mithril-class-test

And a factory component:

https://github.com/spacejack/mithril-factory-test

Experimental typings included. Seems to work okay so far. 👍

Thanks @pygy for implementing #1339, especially for the component factory support. I was using factories for my React "classes" and am looking forward to having the same capability in Mithril. I'll take your code for a spin in my project and let you know if I have any issues.

I've updated my above examples to include POJO, class and factory components here:

https://github.com/spacejack/mithril-component-types

This uses my most recent type definitions for 1.0 with some additons to cover the class and factory component types (see here.) These definitions provide much better inference for POJO and factory components.

Class components are unfortunately a bit awkward to work with. I can't see a way to get strongly typed attrs in the hyperscript function (example) due to the difficulty of expressing the attrs & state types in the parameters. For a class component, hyperscript must take a typeof type which can't be generic.

There are pros and cons to each type of component, and then each of those can utilize different approaches to declare types (or not.)

@pygy has an outstanding PR to fix this in #1339. Just bringing a little more attention to it.

Closing as we are no longer accepting feature requests for 0.2.x.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

designMoreWeb picture designMoreWeb  ·  4Comments

tivac picture tivac  ·  3Comments

mikejav picture mikejav  ·  3Comments

josephys picture josephys  ·  4Comments

volnei picture volnei  ·  3Comments