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

[SUGGESTION] support for custom components on es5 #17825

Closed
lifaon74 opened this issue Aug 16, 2017 · 4 comments
Closed

[SUGGESTION] support for custom components on es5 #17825

lifaon74 opened this issue Aug 16, 2017 · 4 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@lifaon74
Copy link

lifaon74 commented Aug 16, 2017

SUGGESTION

class MyComponent extends HTMLElement {
  constructor() {
    super();
  }
}

Transpiles to :

var MyComponent = (function (_super) {
    __extends(MyComponent, _super);
    function MyComponent() {
        var _this = _super.call(this) || this;
        return _this;
    }
    return MyComponent;
}(HTMLElement));

The _super.call(this) suggests that the super constructor is callable. This isn't true for HTMLElement(s) which are part of the Custom Elements. These requires es6 classes or using Reflect.construct(HTMLElement, [], MyComponent);

A proper es5 should output :

var MyComponent = (function (_super) {
    __extends(MyComponent, _super);
    function MyComponent() {
        var _this = Reflect.construct(_super, [], MyComponent);
       // or var _this = Reflect.construct(_super, [], this.constructor);
       return _this;
    }
    return MyComponent;
}(HTMLElement));

For performances issues, the transpiler should probably only use var _this = Reflect.construct(_super, [], MyComponent); in replacement to _super.call(this) if the mother class is an instance of HTMLElement (done at transpile time, this includes HTMLElement, HTMLImageElement,... and every futures uncallable class constructors).

As an example, this behavior is already present in babel.

A fast fix (not optimized because executing at runtime) could be done though :

var _this = (_super instanceof HTMLElement) ? Reflect.construct(_super, [], this.constructor) : (_super.call(this) || this);
@Jessidhia
Copy link

However, a working Reflect.construct is only available in real ES6 implementations, which kinda defeats the purpose.

@lifaon74
Copy link
Author

@Kovensky Reflect can be polyfilled just like web components and this is what we expect to do when transpiling to es5...
Of course the user should be aware that he requires to add a Reflect polyfill for old platforms. Moreover this functionality should probably be enabled through a flag (--custom-elements) to avoid side effects, ex: where developers manually patch the problem :

try { super(); } catch(e) { this = Reflect.construct(_super, [], this.constructor); }

An adapter is present but only works with es6 browsers... and is slower than a Reflect.construct.

Keep in mind that everything here is suggestion.

@DanielRosenwasser DanielRosenwasser added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Aug 17, 2017
@grimly
Copy link

grimly commented Jan 26, 2018

I would suggest the following implementation :

var MyComponent = (function (_super) {
    __extends(MyComponent, _super);
    function MyComponent() {
        var args = []; // Whatever is used as super constructor arguments
        var _this = _super !== null ? (Reflect && Reflect.construct && Reflect.construct(_super, args, MyComponent) || _super.apply(this, args)) : this;
       return _this;
    }
    return MyComponent;
}(HTMLElement));

The real issue is that we cannot choose to use feature detection and load a polyfill only when custom elements are not implemented. We HAVE to use a polyfill if we want to target ES5 as the compilation output.

@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision and removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Dec 15, 2021
@RyanCavanaugh
Copy link
Member

Given the rarity of people targeting DOM environments with only ES5, this doesn't seem like a good area of investment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants