{"action":"created","issue":{"url":"https://api.github.com/repos/pakastin/redom/issues/4","repository_url":"https://api.github.com/repos/pakastin/redom","labels_url":"https://api.github.com/repos/pakastin/redom/issues/4/labels{/name}","comments_url":"https://api.github.com/repos/pakastin/redom/issues/4/comments","events_url":"https://api.github.com/repos/pakastin/redom/issues/4/events","html_url":"https://github.com/pakastin/redom/issues/4","id":173672001,"number":4,"title":"Feedback","user":{"login":"maciejhirsz","id":1096222,"avatar_url":"https://avatars.githubusercontent.com/u/1096222?v=3","gravatar_id":"","url":"https://api.github.com/users/maciejhirsz","html_url":"https://github.com/maciejhirsz","followers_url":"https://api.github.com/users/maciejhirsz/followers","following_url":"https://api.github.com/users/maciejhirsz/following{/other_user}","gists_url":"https://api.github.com/users/maciejhirsz/gists{/gist_id}","starred_url":"https://api.github.com/users/maciejhirsz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/maciejhirsz/subscriptions","organizations_url":"https://api.github.com/users/maciejhirsz/orgs","repos_url":"https://api.github.com/users/maciejhirsz/repos","events_url":"https://api.github.com/users/maciejhirsz/events{/privacy}","received_events_url":"https://api.github.com/users/maciejhirsz/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":62,"created_at":"2016-08-28T22:35:57Z","updated_at":"2016-08-30T10:15:59Z","closed_at":null,"body":"Gonna do it here instead of mail because formatting code and stuff :).\r\n\r\nNice to see this coming up! I'll definitely enjoy participating on this, I like goofing around with implementing things half way, but I'm still not sure if I'd roll it out as is.\r\n\r\n### Performance\r\n\r\nBenchmark numbers on my hardware:\r\n```\r\nBenching REDOM <div> with multiple child nodes\r\n10692ns per iteration (93529 ops/sec)\r\n```\r\n\r\nThis is a regression from my implementation (it was 6500ns btw, not 5200ns, turns out I had a bug that made all tags into `<div>`s which simplified all the bindings). Passing a function as an argument is powerful, but unless you do it as a reference to a function it also can be costly when you are using curry or other function builders.\r\n\r\nI made a PR with my benchmarking tool thing, poor as it may be. It might be a good idea to test things for performance regressions when deciding whether not a particular feature should make it in.\r\n\r\nMy benchmark was fast because I only used the function call once (to set the class), and it was delegated as the last variant on the `if`/`else` branch as to not slow down text and element nodes, which should make up for the bulk of the markup. The trio of `String` / `Node` / `Function` worked pretty well, which brings me to the next point...\r\n\r\n### Verbosity\r\n\r\n```js\r\n    div(\r\n        h1(setClass('doom'), 'Hello ', b('DOOM'), '!'),\r\n        p('Bacon ipsum dolor amet meatloaf meatball shank porchetta \\\r\n            picanha bresaola short loin short ribs capicola fatback beef \\\r\n            ribs corned beef ham hock.')\r\n    );\r\n```\r\n\r\nBecame:\r\n\r\n```js\r\n    div(\r\n        children([\r\n            h1(className('redom'), text('Hello '), children([\r\n                b(children([\r\n                    text('RE:DOM')\r\n                ]))\r\n            ]), text('!')),\r\n            p(\r\n                text('Bacon ipsum dolor amet meatloaf meatball shank porchetta \\\r\n                    picanha bresaola short loin short ribs capicola fatback beef \\\r\n                    ribs corned beef ham hock.')\r\n            )\r\n        ])\r\n    )\r\n```\r\n\r\nThat's way more verbose. While I typically prefer things explicit over implicit (I do code in Rust after all), I don't particularly like verbosity that doesn't add much, those things belong in the realms of XML and Java 😅.\r\n\r\n- The fact that `text()` is a curry while functions such as `b()` produce an element got me confused since they both are abstractions of the same type of an object - a `Node`. It also means I can't use element functions directly as arguments, and I can't use `text()` calls as children.\r\n- I had to break things into more lines because the sheer number of brackets and parens made it difficult to keep track of things.\r\n\r\n### Components\r\n\r\nI've been goofing around with how to do components with functions for dbmon, my last attempt was this:\r\n```js\r\n    function cell() {\r\n        var elapsed = span();\r\n        var popover = div(setClass('popover-content'));\r\n        var el = td(\r\n            elapsed,\r\n            div(setClass('popover left'), popover, div(setClass('arrow')))\r\n        );\r\n\r\n        function mount(parent) {\r\n            return el;\r\n        }\r\n\r\n        mount.update = function (data) {\r\n            el.className = data.elapsedClassName;\r\n            elapsed.textContent = data.formatElapsed;\r\n            popover.textContent = data.query;\r\n        };\r\n\r\n        return mount;\r\n    }\r\n```\r\n\r\nWhich I'm not very happy with. Having a `this` to bind elements you create like you'd do in FRZR components is very handy, and the functional alternative isn't very clean since variable declarations are statements and not expressions in JS.\r\n\r\nI like your example more:\r\n```js\r\nconst login = form(\r\n  children(el => [\r\n    el.email = input(props({ type: 'email' })),\r\n    el.pass = input(props({ type: 'pass' })),\r\n    el.submit = button(text('Sign in'))\r\n  ]),\r\n  events({\r\n    onsubmit (el, e) {\r\n      e.preventDefault();\r\n\r\n      console.log(el.email.value, el.pass.value);\r\n    }\r\n  })\r\n);\r\n```\r\nBut it has some problems. I'm not a fan of sticking random members on elements - you might get name conflicts if you are unaware, and the concept is not future proof as the standard can expand and something you've been using as a custom member can become a thing in the future, thus introducing breakage.\r\n\r\nActually using something akin to FRZR components would be an option here, with the views being created a function curry: `div(attach(Component))` or `div(mount(Component))`.\r\n\r\n### Other things.\r\n\r\n- I've figured that the function passed in could return a child. This made the `hasChildren` for `textContent` specialization simpler, and can be used to implement the component mounting.\r\n- One idea I had for creating a references would involve a curry such as:\r\n\r\n  ```js\r\n  var el = div('Hello ', ref('name', text('Bar')), '!');\r\n  el.refs.name.textContent = 'World!';\r\n  ```\r\n\r\n  This would not be necessary should we use more FRZR-like components, but it's an idea. It also scopes are custom props to a single object (`refs`) instead of polluting the member space.\r\n\r\n\r\n---\r\n\r\nI can roll out a PR with roughly the same implementation I've had before to see how you like it."},"comment":{"url":"https://api.github.com/repos/pakastin/redom/issues/comments/243395939","html_url":"https://github.com/pakastin/redom/issues/4#issuecomment-243395939","issue_url":"https://api.github.com/repos/pakastin/redom/issues/4","id":243395939,"user":{"login":"pakastin","id":1475902,"avatar_url":"https://avatars.githubusercontent.com/u/1475902?v=3","gravatar_id":"","url":"https://api.github.com/users/pakastin","html_url":"https://github.com/pakastin","followers_url":"https://api.github.com/users/pakastin/followers","following_url":"https://api.github.com/users/pakastin/following{/other_user}","gists_url":"https://api.github.com/users/pakastin/gists{/gist_id}","starred_url":"https://api.github.com/users/pakastin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pakastin/subscriptions","organizations_url":"https://api.github.com/users/pakastin/orgs","repos_url":"https://api.github.com/users/pakastin/repos","events_url":"https://api.github.com/users/pakastin/events{/privacy}","received_events_url":"https://api.github.com/users/pakastin/received_events","type":"User","site_admin":false},"created_at":"2016-08-30T10:15:59Z","updated_at":"2016-08-30T10:15:59Z","body":"Having:\r\n```js\r\nvar Component = view({\r\n  init () {\r\n    this.el = el('component');\r\n  },\r\n  update () {\r\n    ...\r\n  }\r\n});\r\nel(Component);\r\n```\r\n..would make things a bit simplier, right?\r\n\r\nPerformance-wise there's not that much need for a class here I think.."}}
{"actor":{"display_login":"pakastin"}}