{"action":"created","issue":{"url":"https://api.github.com/repos/lhorie/mithril.js/issues/1086","repository_url":"https://api.github.com/repos/lhorie/mithril.js","labels_url":"https://api.github.com/repos/lhorie/mithril.js/issues/1086/labels{/name}","comments_url":"https://api.github.com/repos/lhorie/mithril.js/issues/1086/comments","events_url":"https://api.github.com/repos/lhorie/mithril.js/issues/1086/events","html_url":"https://github.com/lhorie/mithril.js/issues/1086","id":158018925,"number":1086,"title":"[rewrite] vnode structure is difficult to statically optimize","user":{"login":"tivac","id":49545,"avatar_url":"https://avatars.githubusercontent.com/u/49545?v=3","gravatar_id":"","url":"https://api.github.com/users/tivac","html_url":"https://github.com/tivac","followers_url":"https://api.github.com/users/tivac/followers","following_url":"https://api.github.com/users/tivac/following{/other_user}","gists_url":"https://api.github.com/users/tivac/gists{/gist_id}","starred_url":"https://api.github.com/users/tivac/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tivac/subscriptions","organizations_url":"https://api.github.com/users/tivac/orgs","repos_url":"https://api.github.com/users/tivac/repos","events_url":"https://api.github.com/users/tivac/events{/privacy}","received_events_url":"https://api.github.com/users/tivac/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/lhorie/mithril.js/labels/discussion","name":"discussion","color":"d4c5f9"},{"url":"https://api.github.com/repos/lhorie/mithril.js/labels/question","name":"question","color":"cc317c"},{"url":"https://api.github.com/repos/lhorie/mithril.js/labels/rewrite","name":"rewrite","color":"0e8a16"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2016-06-01T22:13:09Z","updated_at":"2016-06-02T03:18:23Z","closed_at":null,"body":"There's a few decisions that have been made about `rewrite`'s vnode structure that is making it tricky to update [`mithril-objectify`](https://github.com/tivac/mithril-objectify/) to work safely & effectively with the `rewrite` branch.\r\n\r\nCurrent WIP is on the [`rewrite`](https://github.com/tivac/mithril-objectify/tree/rewrite) branch, for reference.\r\n\r\n## Text\r\n\r\nIn the `rewrite` text has a couple of different representations\r\n\r\n1. TextNodes use `tag: \"#\"` and set the `children` property the string of text\r\n\r\n    ```js\r\n    m(\"div\", \"a\", \"b\")\r\n\r\n    {\r\n        tag: \"div\",\r\n        ...,\r\n        children: [{\r\n            tag: \"#\",\r\n            ...,\r\n            children: \"a\"\r\n        }, {\r\n            tag: \"#\",\r\n            ...,\r\n            children: \"b\"\r\n        }],\r\n        text: undefined\r\n    }\r\n    ```\r\n\r\n2. Other nodes use `tag: \"<node>\"` and set the `text` property to the string of text\r\n\r\n    ```js\r\n    m(\"div\", \"fooga\")\r\n\r\n    {\r\n        tag: \"div\",\r\n        ...,\r\n        children: undefined,\r\n        text: \"fooga\"\r\n    }\r\n    ```\r\n\r\nSince it can be difficult to statically determine if an argument is a string both of these patterns are tough to optimize.\r\n\r\nAn example from the dbmonster example app shows this well.\r\n\r\n```js\r\nm(\"td\", ..., db.dbname)\r\n\r\n// should become\r\n\r\n{\r\n    tag: \"td\",\r\n    ...,\r\n    text: db.dbname\r\n}\r\n```\r\n\r\nbut because it's unknown what type `db.dbname` is there's no way to optimize it statically.\r\n\r\nIn `mithril@0.2.x` this same line could be be optimized to\r\n\r\n```js\r\n{ tag: \"td\", ..., children: [ db.dbname ] }\r\n```\r\n\r\nsince the vnode structure for text/nodes was more consistent and bare string children were accepted.\r\n\r\n## Children and fragments\r\n\r\nThe change in behavior in `rewrite` where array children are treated as fragments instead of accepting nested arrays makes it difficult to correctly generate an array of children for various invocations of `m()`.\r\n\r\n1. Arguments convert to an array of children nodes\r\n\r\n    ```js\r\n    m(\"a\", m(\"b\"), m(\"c\"))\r\n\r\n    {\r\n        tag: \"a\",\r\n        ...,\r\n        children: [{\r\n            tag: \"b\",\r\n            ...\r\n        }, {\r\n            tag: \"c\",\r\n            ...\r\n        }]\r\n    }\r\n    ```\r\n\r\n2. A single array argument converts to an array of children\r\n\r\n    ```js\r\n    m(\"a\", [ m(\"b\"), m(\"c\") ])\r\n\r\n    {\r\n        tag: \"a\",\r\n        ...,\r\n        children: [{\r\n            tag: \"b\",\r\n            ...\r\n        }, {\r\n            tag: \"c\",\r\n            ...\r\n        }]\r\n    }\r\n    ```\r\n\r\n3. Mixed array and non-array arguments convert to both fragments/direct children\r\n\r\n    ```js\r\n    m(\"a\", [ m(\"b\") ], m(\"c\"))\r\n\r\n    {\r\n        tag: \"a\",\r\n        ...,\r\n        children: [{\r\n            tag: \"[\",\r\n            ...,\r\n            children: [{\r\n                tag: \"b\",\r\n                ...\r\n            }]\r\n        }, {\r\n            tag: \"c\",\r\n            ...\r\n        }]\r\n    }\r\n    ```\r\n\r\nNot knowing if a particular argument is an array or not makes the creation of the correct vnode structure for these types of invocations tough.\r\n\r\nAnother simplified example from dbmonster:\r\n\r\n```js\r\nm(\"tr\", [\r\n    ...,\r\n    db.lastSample.topFiveQueries.map(function(query) {\r\n        return m(\"td\");\r\n    })\r\n])\r\n\r\n// becomes\r\n{\r\n    tag: \"tr\",\r\n    ...,\r\n    children: [\r\n        ...\r\n    , {\r\n        tag: \"[\",\r\n        ...,\r\n        children: [ ... ]\r\n    }]\r\n}\r\n```\r\n\r\nWithout making a leap of logic that any call to `.map()` will return an array (something I'm unwilling to do) there's no way to know that the nested array should be a fragment.\r\n\r\nIn `mithril@0.2.x` this could be optimized to\r\n\r\n```js\r\n{\r\n    tag: \"tr\",\r\n    ...,\r\n    children: [\r\n        ...,\r\n        [ ... ]\r\n    ]\r\n}\r\n```\r\n\r\nThere's probably other instances I'm missing, but these are the two that are most problematic for me right now as I try to do perf testing against the dbmonster application to see if `mithril-objectify` even deserves a future in our brave new `rewrite` future.\r\n\r\nI'm not necessarily advocating for changing these, especially if doing some would hurt run-time perf. I'm fine w/ accepting that the conversion rate will drop w/ `rewrite`. I'd just like to bring up these concerns to see if it's something that can potentially be simplified before the `rewrite` lands in a way that won't affect runtime perf but could improve the ability to statically compile out `m()` invocations.\r\n\r\nCC @lhorie"},"comment":{"url":"https://api.github.com/repos/lhorie/mithril.js/issues/comments/223185509","html_url":"https://github.com/lhorie/mithril.js/issues/1086#issuecomment-223185509","issue_url":"https://api.github.com/repos/lhorie/mithril.js/issues/1086","id":223185509,"user":{"login":"tivac","id":49545,"avatar_url":"https://avatars.githubusercontent.com/u/49545?v=3","gravatar_id":"","url":"https://api.github.com/users/tivac","html_url":"https://github.com/tivac","followers_url":"https://api.github.com/users/tivac/followers","following_url":"https://api.github.com/users/tivac/following{/other_user}","gists_url":"https://api.github.com/users/tivac/gists{/gist_id}","starred_url":"https://api.github.com/users/tivac/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tivac/subscriptions","organizations_url":"https://api.github.com/users/tivac/orgs","repos_url":"https://api.github.com/users/tivac/repos","events_url":"https://api.github.com/users/tivac/events{/privacy}","received_events_url":"https://api.github.com/users/tivac/received_events","type":"User","site_admin":false},"created_at":"2016-06-02T03:18:23Z","updated_at":"2016-06-02T03:18:23Z","body":"Oh, I misunderstood. `Node()` (still should be lower-cased right?) would create the `dom` field but just not expose it via the function arguments. Works for me.\r\n\r\nI can investigate that in a bit, need to step back from this problem for a few and let my eyes uncross."}}
{"actor":{"display_login":"tivac"}}