ibm-js / delite

HTML Custom Element / Widget infrastructure
http://ibm-js.github.io/delite/
Other
68 stars 28 forks source link

regression in build for templates with embedded custom elements #438

Closed wkeese closed 8 years ago

wkeese commented 8 years ago

Templates with embedded widgets no longer work correctly in a built layer. For a template like below, where <my-embedded-widget> has a property called foo:

<template requires=acme/MyEmbeddedWidget>
    <my-embedded-widget foo=bar>
    ...

... the generated code is calling setAttribute():

define("delite/handlebars!acme/MyCompoundWidget.html", [ "acme/MyEmbeddedWidget" ], function() {
    return function anonymous(document, register) {
        var c1 = register.createElement("my-embedded-widget");
        c1.setAttribute("foo", "bar");

But, it should be setting the property directly:

c1.foo = "bar";

(Note that this wouldn't be a critical issue if we had attribute reflection as mentioned in #33. But we don't. And even if we did, it would still be somewhat of a performance issue.)

This is a regression from #432.

wkeese commented 8 years ago

For Template.js to work properly, it needs to instantiate a dummy copy of <my-embedded-widget> and introspect what the native properties are. This isn't happening during a build. Furthermore, it's a bit problematic to do during a build, because that requires that all the custom element code (i.e. MyEmbeddedWidget.js) load correctly on Node. This doesn't happen naturally because symbols like HTMLElement aren't defined in Node. You can get a definition of HTMLElement from the "jsdom" package but it seems like an uphill battle.

The compilation of template HTML into the JSON-esque AST (as documented on http://ibm-js.github.io/delite/docs/0.8.1/Template.html) works fine on the server. The problem is the second stage, compiling down to javascript. That second stage is also somewhat questionable since the footprint of a template in javascript form is bigger than the footprint in AST form. I think in this case discretion is the better part of valour and it makes sense for the build to just compile down to the AST.

Another possible change would be to rewrite Template.js to not do code generation, but rather instead to render from an optimized version of the AST, something like:

{
    // attributes to set, like this.setAttribute("class", ...)
    attributes: {
        className: {
                      value: function () { return "d-reset" + this.baseClass },
                      dependsOn: ["baseClass"]
                },
        ...
    },

    // properties to set, like this.foo = 123
    properties: {
        className: {
                      value: function () { return this.bar },
                      dependsOn: ["bar"]
                },
        ...
    },
    ...
}

Again, that would be a trade off between the performance of instantiating a template vs. the code size of Template.js.