MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
14.02k stars 925 forks source link

Rename m.module to m.component #497

Closed gilbert closed 9 years ago

gilbert commented 9 years ago

Everyone else uses "component" (React, Knockout, Web Components, etc) source comment

the concept of a component as a UI piece is well understood and unambiguous, broadly agreed on, and closely or directly matches Mithril's conception source comment

l-cornelius-dol commented 9 years ago

This has my strong and unequivocal vote.

tinchoz49 commented 9 years ago

:+1:

Draggha commented 9 years ago

Avoiding confusion and increasing clarity is always a good thing for documentations. +1

gilbert commented 9 years ago

@lhorie, if you're ok with this change, do you think it could be included in the components release? It will make teaching Mithril's component features a lot easier (for me at least).

lhorie commented 9 years ago

This change is trivial, but before I commit to it, is everyone cool w/ typing out "m.component" all the time or does anyone have a shorter suggestion? (yes, I realize this is a bikeshedding question)

oren commented 9 years ago

:+1:

l-cornelius-dol commented 9 years ago

Thumbs up from me; I value clarity over brevity in a public API.

It would be good to add m.mount at the same time, per #498, leaving m.module to be removed in version 1.0, so that m.component is not overloaded.

gilbert commented 9 years ago

@lhorie All cool here. It's trivial for devs to do something like m.comp = m.component in their own projects. I think the longer, but clearer name will improve the docs highly.

@lawrence-dol That's an interesting way to introduce the change while keeping m.module backwards compatible. I like it.

lhorie commented 9 years ago

@lawrence-dol yep that's the plan

markmarijnissen commented 9 years ago

Wow I finally understand that module is overloaded to both mount and do components. Definately do this! +1

l-cornelius-dol commented 9 years ago

@lhorie, By the way, is the intent to do:

m.mount(dom,m.component(aComponent));

or

m.mount(dom,aComponent);

or both?

I'm not entirely clear on the use-case of (soon to be) m.component. Is it now required in views for subcomponents?

gilbert commented 9 years ago

@lawrence-dol Both are valid, according to #498

lhorie commented 9 years ago

@lawrence-dol m.component is only required if you want to pass arguments to the component. If you call m.component without arguments, it'll return a component that is equivalent to the input component

m("div", [
  //these two are the same thing
  Foo,
  m.component(Foo),

  //this one takes a parameter
  m.component(Bar, {test: "hello world"})
])
lhorie commented 9 years ago

@mindeavor brought up a interesting idiom on the chat:

//the helper
var component = function () {
  var comp = function (props, content) {
    return m.module(comp, props, content)
  }
  return comp
}

//a sample component
var MyComponent = component()
component.controller = function() {...}
component.view = function() {...}

//component usage:
m("div", [
  //with arguments
  MyComponent({id: 123}),

  //without arguments, these two are equivalent
  MyComponent(),
  MyComponent
])

I'm thinking m.component could look like that. Then the splat in m.module(root, mod, ...args) can be dropped since modules would always be parameterized on their own

StephanHoyer commented 9 years ago

This looks really slick...

Would love to see this.

Although it might be usefull to give controller and view upfront

//a sample component
var MyComponent = component({
  controller: function() {...}
  view: function() {...}
})
barneycarroll commented 9 years ago

@mindeavor brought up a interesting idiom on the cha

When I read that at the time I thought either it's pseudo-code I'm not grokking or there's been a typo with one of the references. Only just got it now — ingenious @mindeavor! :)

A couple of points:

Asking users to extend a functor's initial output with methods, and having the inner function read those keys is pretty 'black magic'. It might be easier to get people to swallow a simple factory that consumed standard component static objects:

var component = function( comp ) {
  return function (props, content) {
    return m.module( comp, props, content)
  }
}

//a sample component
var MyComponent = component( {
  controller : function() {...},
  view : function() {...}
} );

Usage is the same. This looks simpler and follows more conventional Javascript techniques for initializing objects with special properties.

I'm kinda shooting myself in the foot with that recommendation, because partially applying { controller, view } objects means their constituent parts are no longer ostensible in application space. The flip side is that you could still use 'unapplied' old-school components using Modulator syntax in your app:

component( comp )( content );
barneycarroll commented 9 years ago

For posterity, @mindeavor suggested this idea here in the chatroom, linking to this gist

barneycarroll commented 9 years ago

I took a further stab at this and came up with an extension of @mindeavor's patterns that allows easy ( { controller, view } ) invocation while attaching all of the passed in component's properties to the returned function. This has the crucial benefit of not invalidating any of the examples in the official reference where the component object has custom properties that are referenced by controller and view at run-time (note there have been interesting conversations simmering recently as to whether the controller object structure could be extended with configuration hooks and alternative usage API methods this way):

m.comp = function( component ){
    var applied = function(){
        m.module.apply( undefined, arguments );
    };

    // Object.assign( applied, conmponent );
    for( var key in component ){
        applied[ key ] = component[ key ];
    }

    return applied;
}

If something like this made it into core, I'd suggest making what is referenced as m.module in the examples above a private function in Mithril. Exposing it would just add confusion.

sloria commented 9 years ago

@barneycarroll Asking users to extend a functor's initial output with methods, and having the inner function read those keys is pretty 'black magic'.

Ha, came to the same conclusion while I was playing around with ways to get react-style components in mithril: https://gist.github.com/sloria/bbdf5e2fe3ae5bf96d99 . I like @barneycarroll 's tweak above as well to account for custom properties.

If something like this made it into core...

I'm not sure this needs to be into core. There are infinite ways users might want to tweak the function for their needs. Perhaps a sensible policy would be to "bring your own sugar" (BYOS, haha).

barneycarroll commented 9 years ago

I'm not sure this needs to be into core. There are infinite ways users might want to tweak the function for their needs. Perhaps a sensible policy would be to "bring your own sugar" (BYOS, haha).

@sloria this is a very good way of framing the conversation. However I believe that Mithril billing itself as A Javascript Framework for Building Brilliant Applications and not providing the most convenient possible API for nesting component X inside component Y would be disingenuous. As with other convenience & flexibility proposals for core, I think the onus should be on the internals to justify why such things aren't desirable.

lhorie commented 9 years ago

To clarify, the idea for components-are-functions is that the component function would be the only way of creating components, which implies m.mount(domElement, module, ...args) would be reverted back to its current m.mount(domElement, module) signature, and the m.component(module, ...args) signature would be dropped in favor of var a = m.component(module); a(...args).

You'd still be able to use old school {controller, view} modules, but those would be considered deprecated. The old school modules are still largely compatible, with the exception that they are cumbersome to parameterize (and if you did want to make them parameterizable, you'd just pass it through the component function to turn it into a "real" component)

The biggest syntactical win for components-are-functions is that you only call m.component once when you define the component, rather than having to type out m.component every time you use it. It also groups the responsibility of taking argument with the component itself, as opposed to deferring this responsibility to the m.component call. I think those two are very strong points in favor of components-are-functions.

The downsides are that it does look magical for the untrained eye (even though the implementation is quite trivial), and it's a betrayal to the idea that Mithril doesn't provide "base classes".

l-cornelius-dol commented 9 years ago

OK, I am going to risk looking foolish here and ask, how these are equivalent:

MyComponent(),
MyComponent

doesn't the first translate to passing (from the composing function) the module return:

m.module(comp, props, content)
// Mithril treats `comp` as the module

and the second to passing the anonymous function:

function (props, content)
// Mithril treats `<anon>` as the module

???

lhorie commented 9 years ago

@lawrence-dol I mean that they are functionally equivalent (although not referentially identical)

sloria commented 9 years ago

...not providing the most convenient possible API for nesting component X inside component Y would be disingenuous

... you only call m.component once when you define the component... It also groups the responsibility of taking argument with the component itself

Yep, I can get behind that.

The downsides are that it does look magical for the untrained eye (even though the implementation is quite trivial), and it's a betrayal to the idea that Mithril doesn't provide "base classes".

Forgivable. There isn't any inheritance going on, and I don't really consider component to be a "base class". After all, it's just a function that returns a function.

barneycarroll commented 9 years ago

After all, it's just a function that returns a function.

m.prop, m.request… Yesterday's core convenience is today's "I don't understand it and I hate it". It will be interesting to work out Mithril's target market shift across patch releases.

l-cornelius-dol commented 9 years ago

As usual JavaScript allows 1001 possibilities for the (almost) same result. I think what's being overlooked in all of this is the fundamental definition of a Mithril component.

If we consider that Mithril needs to create controllers and invoke view-generator functions as needed, then it seems to me that the current "old school" module definition is correct as far as what's given to Mithril is concerned, and a "component" should lock that in and it should be as simple as possible:

// logically a template which results in multiple instances of 
// `Controller` all sharing the one `view` function.
var applicationComponent={
    Controller: /* a constructor */
    view: /* a function returning a v-dom map */
 };

A component _is not an instance_ of itself; logically the "instance", such as it is, is an instance of the component's controller, constructed on demand by Mithril; the "component" then, is most closely aligned to the traditional concept of a "module", but is not necessarily a JavaScript module - a JavaScript module might contain many components. If a component is used in multiple places, each use has a unique controller instance, but the same view function.

Then, m.component is the way, and the only way, to create whatever Mithril needs internally, which could be a simple clone/copy of the two fields, but which is opaque to the application:

var mithrilComponent=m.component(applicationComponent);
// now mithrilComponent is something that can be used in m() and
// m.mount. The application doesn't know about it's internals.

How an application component is created is entirely up to the application:

// most basic
var appComp={ Controller: XyzController, view: XyzView.view; };

// constructor with bound values
var appComp=new XyzComponent(abc,def,...);

// factory function with bound values
var appComp=XyzFactory.create(abc,def,...);

Regardless, use of the application component is always the same:

var mthComp=m.component(appComp);

// mount it
m.mount(domNode,mthComp);

// in a view
function view(ctl) {
    m("div", [
        mthComp,
        anotherMthComp,
    ]); 
}

Giving it access to the parent's controller should be done by creating it in the parent's controller:

// parent
function Controller() {
    this.subComp=m.component(new FancyComponent(this /*...*/));
}

// parent's view:
function view(ctl) {
    m("div", [
        ctl.subComp, // can access the parent's controller
    ]); 
}

With the question remaining being, should Mithril treat the result of m.module as a function to be invoked, or an object to be passed? The former allows injection of arguments by the actual view:

// parent's view:
function view(ctl) {
    m("div", [
        ctl.subComp1, // this?
        ctl.subComp2(), // or this? Allowing e.g. subComp2("Hello")
    ]); 
}

But it seems cleaner to extract any per-view values from the controller and leave the views with but a single argument -- their _own_ controller -- therefore favoring the first form of using the component, which passes the component to m and m.mount as a simple property.

lhorie commented 9 years ago

@lawrence-dol ok I see where you're coming from. All I was saying was that with that particular implementation, the two idioms had the same effect when their codes run.

If the implementation of m.component was, for example, var Callable = m.module.bind(m, aComponent), then there would be no possibility for the "components being instances of themselves" interpretation, since the output would be an opaque function that could only take arguments and nothing else. Incidentally, this opaque function is the concept that I was telling @barneycarroll I was having a hard time explaining to a rubber duck.

In any case, the semantics of components-are-functions are that m.component "extends" an object in such a way that the base object that holds the controller and view becomes callable, and that it binds arguments (in the Function.prototype.bind sense of the word) to its member functions when called. Whether controller and view are public members of the function that is returned by m.component is irrelevant for most purposes, imho.

gilbert commented 9 years ago

I think it's good to have controller and view in the public output, but only demonstrate the "extend" version in documentation. Having those properties available will give great flexibility for plugins and helper functions.

barneycarroll commented 9 years ago

Whether controller and view are public in the output of m.component is irrelevant, imho

Extending bound / live components doesn't have much of a use case, AFAICT. And I would bring it up if I thought there was one ;)

Incidentally, this opaque function is the concept that I was telling @barneycarroll I was having a hard time explaining to a rubber duck.

Glad you brought it up. The difference is very significant though: under the proposal above, that opaque function is what users get from 3rd party components. Debugging, inspection, extension, become impossible. Modulator definitely doesn't want to pass around these opaque functions as first class citizens since under that API the results is a module component whose instantiation parameters have been configured. You would always want to do this at or near the calling context.

gilbert commented 9 years ago

Oh, I think I misunderstood. I thought we were talking about m.component returning a function, as opposed to exposing the instantiated controller + view. Disregard my previous comment.

l-cornelius-dol commented 9 years ago

@mindeavor, @barneycarroll, @lhorie : Forgive me if I am misunderstanding you, but if you are referring to my example, and only to be clear, I think m.component should not return an already instantiated controller and a view generator, but an object (conceptually opaque to the application) which contains a controller constructor and a view generator function.

In other words, the output is a new object that is exactly what is called a "module" today in Mithril. But the input is any object which exposes those two things. Logically this represents what is typically called a module in general programming terminology, and the correlation to a general OO "instance" of a component is a pair, internal to Mithril, of an instantiated controller and the (singular) view generator function.

(And I only refer to a constructor out of pragmatism; I still maintain that a controller creator function and a view creator function are the right pair of functions that should constitute a component.)

lhorie commented 9 years ago

@lawrence-dol I think the misunderstanding is one of OOP vs FP. You keep talking about instantiating, which implies your understanding of it is var componentInstance = m.component(Component), but it's more similar to a functional-style curry, i.e. m.component :: x -> args -> x'

Here's an example of how it would work, if it makes things clearer:

var Foo = m.component({
  controller: function(args) {
    console.log(args)
  },
  view: function(ctrl, args) {
    return m("div", args.test)
  }
})

new Foo.controller({test: 123}) // logs {test: 123}
m.mount(document.body, Foo) //<div></div>

var FooWithArgs = Foo({test: 555}) //does not log or render anything
new FooWithArgs.controller() //logs {test: 555}
m.mount(document.body, FooWithArgs) //logs {test: 555} and renders <div>555</div>

var FooWithoutArgs = Foo() //does not log or render anything
new FooWithoutArgs.controller({test: 234}) //logs {test: 234}
m.mount(document.body, FooWithoutArgs({test: 345})) //logs {test: 345} and renders <div>345</div>

//typical usage
m.mount(document.body, Foo({test: 789})) //logs {test: 789} and renders <div>789</div>
l-cornelius-dol commented 9 years ago

@lhorie : I think I am slowly getting there; thanks for your patience.

So the only, or at least main, purpose of m.component is to curry arguments onto the controller and view functions, that is to perform partial application?

To me the your examples have inconsistencies or weirdness to them.

First, there's the use of leading capitals for functions which are not ctors and are not invoked with new ..., which signals to my brain a bug.

Second, there's the conflicting ..., FooWithoutArgs) and ..., FooWithoutArgs(...)) which to me means in one case we've passed a function as a value and in the second passed the return of that function -- this looks like one case must be a bug. That should not be allowed, never mind idiomatic -- it just plain looks like a mistake.

I think it would make more sense if the use-case were always:

var foo=m.component(...); // lead-lower; a *function*, not a *Ctor*
m.mount(node,foo()); // with or without arguments
m("div", [ foo() ]); // with or without arguments

and both m and m.mount threw an error if the component passed specifically _is_ a function returned by m.component (rather than its invocation), even if that means m.component marks the function with a flag or similar.

lhorie commented 9 years ago

I think it would make more sense if the use-case were always

Yeah, that's what the "typical usage" line does. The rest of the examples just happens to work the way they do because of how it's implemented, but typically you wouldn't be monkeying around with those kinds of idioms. I just posted them for the sake of completeness.

l-cornelius-dol commented 9 years ago

@lhorie

The rest of the examples just happens to work...

If it were up to me, I'd explicitly make them not work.

barneycarroll commented 9 years ago

@lawrence-dol you have a good point. This is kind of what I'm arguing with Modulator — AFAICT the components branch is hacky in its fundamentals. When it comes to reasoning about the identity question, Modulator says "that's actually a really tricky problem and needs a holistic solution" whereas components branch says "hey we already kind of do that with DOM nodes". On a less biased front, this definitely reduces the strength of the "don't do stuff that looks like magic" / rubberduck every API argument.

gilbert commented 9 years ago

Ok, I take it that everyone is ok with renaming m.module to m.component, so this issue can be considered resolved. However this turned into a discussion about changes to the semantics of m.component. For this, I aggregated the code examples and created a new issue #530

lhorie commented 9 years ago

m.component landed in the components branch, so I'm closing this. Semantics discussions should be moved to #530