uhop / dcl

Elegant minimalistic implementation of OOP with mixins + AOP in JavaScript for node.js and browsers.
http://www.dcljs.org/
Other
76 stars 10 forks source link

can't extend HTMLElement #7

Closed wkeese closed 11 years ago

wkeese commented 11 years ago

You cannot create a class that extends HTMLElement. This precludes using DCL to create web components (see the "Basic usage" section of http://www.polymer-project.org/platform/custom-elements.html).

A simple test case (to be placed in the dcl/tests directory) is:

<!DOCTYPE html>
<html>
    <head>
        <script src="http://cdnjs.cloudflare.com/ajax/libs/es5-shim/1.2.4/es5-shim.min.js"></script>
        <script src="http://requirejs.org/docs/release/2.1.5/minified/require.js"></script>
        <script>
            require(["../dcl"], function(dcl){
                var ExtendedElement = dcl(HTMLElement, {
                    constructor: function(){
                        console.log("extended constructor");
                    }
                });
                var eei = new ExtendedElement();
                debugger;
            });
        </script>
    </head>
    <body>
    </body>
</html>

It will hit an exception and pause in the browser's debugger, assuming that the debugger is open.

Cc @kitsonk who originally mentioned this issue.

uhop commented 11 years ago

Is it possible to post the exception and what browser was used?

wkeese commented 11 years ago

Oh, it's happening (at least) on Chrome, with the exception in mini.js in the stubChain function:

for(var i = 0; i < l; ++i){
    chain[i].apply(this, arguments);
}

Chain[0] is the HTMLElement, and you can reproduce the root of the problem in the console:

HTMLElement()
> TypeError: Illegal constructor
uhop commented 11 years ago

I looked into that. The problem is that HTMLElement is:

Obviously, it is possible to accommodate such constructs, the real question is how? Treat it as a special case? Create a registry for founding "classes" like that? Do we need to add a special helper, which will automatically register such "classes" with a document?

I am leaning towards a registry with HTMLElement and related "classes" pre-registered, framed as a separate HTML-specific module added on demand, but if you guys have better ideas, I would be glad to hear them.

PS: A side note: what Polymer guys were thinking bypassing normal JavaScript ways to do things? My guess: they complained about restrictiveness of existing implementations, which they decided to enshrine for a foreseeable future. :-(

wkeese commented 11 years ago

I guess any of those options would work. A registry sounds fine.

Another approach is to ignore "Illegal constructor" TypeErrors altogether. You'd need a try/catch around each constructor call though so it might affect performance. You could also build a "registry" on the fly by checking each constructor the first time you see it.

Note that with web components we will never call new on the class returned from dcl; we just want it for the prototype. I hope that doesn't flummox the internal workings of dcl?

BTW for some reason this isn't an issue with ComposeJS, so you might want to check why it works there.

uhop commented 11 years ago

IIRC ComposeJS does not chain constructors automatically, so it is never called.

wkeese commented 11 years ago

Cool, looks like you fixed this in your 1.1 release; that the dcl() call runs without an error. For the record, my updated test case is:

    <!DOCTYPE html>
    <html>
            <head>
                    <script src="http://requirejs.org/docs/release/2.1.5/minified/require.js"></script>
                    <script>
                            require(["../dcl"], function(dcl){
                                    var SuperButton = dcl(HTMLElement, {
                                            foo: function(){
                                                    console.log("foo method");
                                            }
                                    });
                                    console.log("appendChild: ", SuperButton.prototype.appendChild);
                                    console.log("foo: ", SuperButton.prototype.foo);
                            });
                    </script>
            </head>
            <body>
            </body>
    </html>
uhop commented 11 years ago

I still have to check if it is enough.

wkeese commented 11 years ago

OK. Well anyway it seems to be working for us (see latest code on https://github.com/ibm-dojo/dui)

Incidentally I worked around the lack of native accessor support by implementing it myself in our Stateful class.