Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't extend HTMLElement #7

Closed
wkeese opened this issue Sep 10, 2013 · 8 comments
Closed

can't extend HTMLElement #7

wkeese opened this issue Sep 10, 2013 · 8 comments
Assignees
Milestone

Comments

@wkeese
Copy link

wkeese commented Sep 10, 2013

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
Copy link
Owner

uhop commented Sep 10, 2013

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

@wkeese
Copy link
Author

wkeese commented Sep 10, 2013

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

@ghost ghost assigned uhop Sep 14, 2013
@uhop
Copy link
Owner

uhop commented Sep 14, 2013

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

  • a function
  • yet it is not a valid constructor
  • yet it has a valid prototype, which should be used with an empty constructor
    • It may have a non-trivial constructor, which runs behind the scenes -- we don't really know because there is no way to create an element directly with pure JavaScript facilities without registering a prototype with a document, which returns a constructor function.

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
Copy link
Author

wkeese commented Sep 14, 2013

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
Copy link
Owner

uhop commented Sep 14, 2013

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

@wkeese
Copy link
Author

wkeese commented Oct 6, 2013

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>

@wkeese wkeese closed this as completed Oct 6, 2013
@uhop
Copy link
Owner

uhop commented Oct 7, 2013

I still have to check if it is enough.

@wkeese
Copy link
Author

wkeese commented Oct 11, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants