JSX second null argument compatibility fix #772

avesus · 2015-08-20T13:00:20Z

Babel and other JSX compilers pass 'null' value as second argument
to m() function if there is no attributes in element specified.
Added check for 'null' to support JSX compilers transparently.

NOTE that passing children elements as arguments is already supported.

dead-claudia · 2015-08-22T06:53:13Z

mithril.js

-			args[i - 1] = arguments[i];
-		}
+		// JSX compatibility with 'null' {pairs} argument
+		var args = Array.prototype.slice.call(arguments, (pairs === null ? 2 : 1));


Nit: just set i to pairs === null ? 2 : 1. Otherwise, you're creating a perf regression.

(Chrome and Firefox both don't usually like it when you pass the arguments object as an parameter. This is one case.)

Thank you, I'll fix it.
On Sat, Aug 22, 2015 at 9:53 AM Isiah Meadows notifications@github.com
wrote:

In mithril.js
#772 (comment):

@@ -49,9 +49,9 @@ var m = (function app(window, undefined) {
*
*/
function m(tag, pairs) {

for (var args = [], i = 1; i < arguments.length; i++) {

args[i - 1] = arguments[i];

}

// JSX compatibility with 'null' {pairs} argument

var args = Array.prototype.slice.call(arguments, (pairs === null ? 2 : 1));

Nit: just set i to pairs === null ? 2 : 1. Otherwise, you're creating a
perf regression.

(Chrome and Firefox both don't usually like it when you pass the arguments
object as an parameter. This is one case.)

—
Reply to this email directly or view it on GitHub
https://github.com/lhorie/mithril.js/pull/772/files#r37693412.

Babel and other JSX compilers pass 'null' value as second argument to m() function if there is no attributes in element specified. Added check for 'null' to support JSX compilers transparently. NOTE that passing children elements as arguments instead of Array is already supported.

avesus · 2015-08-24T13:13:31Z

@lhorie is this useful?

avesus · 2015-08-28T13:41:20Z

Actually supporting null second argument to m() makes Babel to compile JSX directly into beautiful Mithril code! I use this PR in my project and things work like a magic. I prefer pure JS code, but my partner reads better JSX templates, and working on a personal project late night allows to move on not "parsing" opened and closed braces and not counting commas.

I think that lot of Mithril supporters hate JSX. But transparently supporting it makes Mithril ultra-popular.

Shure, transparently supporting of JSX and some onfocus/onblur preserving parts for editboxes makes Mithril obsolete React.

dead-claudia · 2015-08-28T14:42:32Z

Not everyone in the Mithril community hates JSX, or we wouldn't have MSX.

On Fri, Aug 28, 2015, 09:41 Ivan Borisenko notifications@github.com wrote:

Actually supporting null second argument to m() makes Babel to compile
JSX directly into beautiful Mithril code! I use this PR in my project and
things work like a magic. I prefer pure JS code, but my partner reads
better JSX templates, and working on a personal project late night allows
to move on not "parsing" opened and closed braces and not counting commas.

I think that lot of Mithril supporters hate JSX. But transparently
supporting it makes Mithril ultra-popular.

Shure, transparently supporting of JSX and some onfocus/onblur preserving
parts for editboxes makes Mithril obsolete React.

—
Reply to this email directly or view it on GitHub
#772 (comment).

avesus · 2015-08-28T14:44:44Z

Guys from the MSX team said that they will deprecate MSX in favor of Babel,
if I understand them right.
On Fri, Aug 28, 2015 at 5:42 PM Isiah Meadows notifications@github.com
wrote:

Not everyone in the Mithril community hates JSX, or we wouldn't have MSX.

On Fri, Aug 28, 2015, 09:41 Ivan Borisenko notifications@github.com
wrote:

Actually supporting null second argument to m() makes Babel to compile
JSX directly into beautiful Mithril code! I use this PR in my project and
things work like a magic. I prefer pure JS code, but my partner reads
better JSX templates, and working on a personal project late night allows
to move on not "parsing" opened and closed braces and not counting
commas.

I think that lot of Mithril supporters hate JSX. But transparently
supporting it makes Mithril ultra-popular.

Shure, transparently supporting of JSX and some onfocus/onblur preserving
parts for editboxes makes Mithril obsolete React.

—
Reply to this email directly or view it on GitHub
#772 (comment).

—
Reply to this email directly or view it on GitHub
#772 (comment).

dead-claudia · 2015-08-28T14:51:45Z

I was talking about part of the community, not the MSX compiler itself. I
still like this patch, by the way.

On Fri, Aug 28, 2015, 10:44 Ivan Borisenko notifications@github.com wrote:

Guys from the MSX team said that they will deprecate MSX in favor of Babel,
if I understand them right.
On Fri, Aug 28, 2015 at 5:42 PM Isiah Meadows notifications@github.com
wrote:

Not everyone in the Mithril community hates JSX, or we wouldn't have MSX.

On Fri, Aug 28, 2015, 09:41 Ivan Borisenko notifications@github.com
wrote:

Actually supporting null second argument to m() makes Babel to compile
JSX directly into beautiful Mithril code! I use this PR in my project
and
things work like a magic. I prefer pure JS code, but my partner reads
better JSX templates, and working on a personal project late night
allows
to move on not "parsing" opened and closed braces and not counting
commas.

I think that lot of Mithril supporters hate JSX. But transparently
supporting it makes Mithril ultra-popular.

Shure, transparently supporting of JSX and some onfocus/onblur
preserving
parts for editboxes makes Mithril obsolete React.

—
Reply to this email directly or view it on GitHub
<#772 (comment)
.

—
Reply to this email directly or view it on GitHub
#772 (comment).

—
Reply to this email directly or view it on GitHub
#772 (comment).

avesus · 2015-08-28T14:59:53Z

insin/msx#13 (comment)
Note that MSX will be deprecated as soon as the next version of Mithril is released, as a change has been made which allows you to make full use of Babel's JSX transpiler instead. Babel will pass any children as additional arguments, so you'd need to use this signature instead to put children into an array.

But he's not mentioned that secondary null argument is the problem while. Because talk was about upcoming next version.

dead-claudia · 2015-08-28T16:53:57Z

mithril.js

 		}
+
 		if (isObject(tag)) return parameterize(tag, args);


Nit: shouldn't pairs be sent as-is as well to parametrize, in the form of the first part of args? Something like this?

for (var args = [], i = 0; i < arguments.length; i++) { args.push(arguments[i]); } if (isObject(tag)) return parameterize(tag, args); if (pairs === null) args.shift();

It's a breaking change otherwise.

I think we should review all cases:

/// Base Mithril cases: // arguments.length === 1, pairs is undefined m('div') // arguments.length === 2+, pairs is Object, next arguments is undefined or Array (the single third argument) or one or more functions m('div', {id: '1'}, ...) // arguments.length === 2, pairs is Array m('div', [m('div')]) // arguments.length === 2+, pairs is function, as next arguments should too m('div', m('div')) // arguments.length === 2+, pairs is String m('div', 'Hello')

JSX emitter always emits function call with 2 or 3 arguments. In case of absent pairs, it emits null instead.
So for JSX, cases are following:

m('div', {}) m('div', null, m('div')) m('div', {}, m('div'))

So, if second argument is null, Mithril just could consider that second argument is not second, but third one.
I'll fix the PR.

This solution would suffice, right?

function m(tag, pairs) { var i = 1; // Fix JSX style passing if (arguments.length > 2 && pairs === null) { i = 2; pairs = arguments[2]; } for (var args = []; i < arguments.length; ++i) { args.push(arguments[i]); } if (isObject(tag)) return parameterize(tag, args); // ... }

dead-claudia · 2015-08-28T17:07:15Z

@avesus Please add unit tests for this. Here's what I see is needed with this (at least):

m(component, args, "foo") and m.component(component, args, "foo"), when the controller is called, should both call it with args and "foo"
m(component, null, args) and m.component(c1, null, args) when the controller is called, should both call it with null and args (i.e. they should be passed verbatim).
m("div", null, "body") and m("div", "body") should return an equivalent node.

avesus · 2015-08-28T20:59:51Z

@IMPinball not shure why consider m.component(c1, null, args) in tests. Could you explain?

Tests are very important part, I hope to write them on the next week. Have a nice weekend, thanks for the good peer review!

dead-claudia · 2015-08-28T22:41:26Z

@avesus

You can never test code too much.
See my line comment explaining where the current PR could lead to unexpected behavior with components.

var Foo = {
  view: function (ctrl, opts) {
    opts = opts || {}
    var children = [].slice.call(arguments, 2).map(function (value, i) {
      return m('div', 'Index: ' + i, 'Body: ', value)
    });
    return m('div', [opts.header || "A list"].concat(children))
  },
};

// This will weirdly throw an error in the view itself.
m(Foo, null, [m('span', 1)]);
``

avesus · 2015-09-10T17:03:46Z

@IMPinball sorry for my long wait. After dive deep into Mithril's source code and meeting with s-n-a-b-b-d-o-m (with dashes to not promote it here like an ad) I wonder about alternatives.
Is Mithril really slower by 20 times to the s-worded alternative and how to make it faster? Also, I like modular approach but it is not Mithril' choice :-(. So, my motivation right now becomes much lower after discovering the alternative. But less and faster is not necessary better. Thus I still consider Mithril as important candidate as the base for my projects.

But my aaannnsweeeer could be very slow henceforth.

Feel free to bring this task to completion. I'l write when (if) come back!

dead-claudia · 2016-02-04T05:26:36Z

@avesus You mind redoing this against next? It's an easy patch, although breaking for a single case.

dead-claudia · 2016-08-29T08:09:06Z

Closing due to inactivity. If you feel this should still be done, please feel free to file a new issue and/or pull request. 😄

avesus mentioned this pull request Aug 20, 2015

Transparent support of JSX syntax with Babel #768

Closed

dead-claudia reviewed Aug 22, 2015
View reviewed changes

avesus force-pushed the master branch from 132a7ed to 7ebce6e Compare August 24, 2015 13:00

avesus force-pushed the master branch from 7ebce6e to 3a3cf79 Compare August 24, 2015 13:04

dead-claudia reviewed Aug 28, 2015
View reviewed changes

dead-claudia mentioned this pull request Aug 28, 2015

Is there a JSX equivalent to this.props.children? insin/msx#13

Open

avesus mentioned this pull request Aug 28, 2015

Migration to Babel sectore/mithrilify#4

Open

dead-claudia closed this Aug 29, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSX second null argument compatibility fix #772

JSX second null argument compatibility fix #772

avesus commented Aug 20, 2015

dead-claudia Aug 22, 2015

avesus Aug 22, 2015

avesus commented Aug 24, 2015

avesus commented Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Aug 28, 2015

dead-claudia Aug 28, 2015

avesus Aug 28, 2015

avesus Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Sep 10, 2015

dead-claudia commented Feb 4, 2016

dead-claudia commented Aug 29, 2016

JSX second null argument compatibility fix #772

JSX second null argument compatibility fix #772

Conversation

avesus commented Aug 20, 2015

dead-claudia Aug 22, 2015

Choose a reason for hiding this comment

avesus Aug 22, 2015

Choose a reason for hiding this comment

avesus commented Aug 24, 2015

avesus commented Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Aug 28, 2015

dead-claudia Aug 28, 2015

Choose a reason for hiding this comment

avesus Aug 28, 2015

Choose a reason for hiding this comment

avesus Aug 28, 2015

Choose a reason for hiding this comment

dead-claudia commented Aug 28, 2015

avesus commented Aug 28, 2015

dead-claudia commented Aug 28, 2015

avesus commented Sep 10, 2015

dead-claudia commented Feb 4, 2016

dead-claudia commented Aug 29, 2016