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

Unable to extend DOM classes at runtime #10063

Open
WebReflection opened this issue Jun 6, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@WebReflection
Copy link

commented Jun 6, 2019

Bug Report

Current Behavior
If an extend is created at runtime, hence without an explicit class, it's impossible to instantiate any instance later on with such class.

Input Code
The following code works out of the box in Chrome and Firefox, but it fails only with the second class once transpiled. See the console error in Babel site itself (P.S. you cannot define twice the same component, so if you edit the code, be sure you also refresh the page before making any conclusion/assumption)

class OK extends HTMLParagraphElement {}
customElements.define('is-ok', OK, {extends: 'p'});
try { new OK; } catch(o_O) {
  console.error('OK', o_O);
}

class Fail extends document.createElement('p').constructor {};
customElements.define('it-fails', Fail, {extends: 'p'});
try { new Fail; } catch(o_O) {
  console.error('Fail', o_O);
}

Expected behavior/code
Since document.createElement('p').constructor is exactly the class HTMLParagraphElement, and since extending explicitly HTMLParagraphElement has no issues, I'd expect Babel to not throw also when extending the retrieved class, as it is natively in the browser.

Babel Configuration (.babelrc, package.json, cli command)
The previously posted REPL configuration has es2015 on ... that's it.

Environment

  • Babel version(s): REPL says 7.4.0
  • How you are using Babel: any way I've tried produced the same error

Possible Solution
The only difference between the two extends is that _wrapNativeSuper(HTMLParagraphElement) is not used for the Fail class, so that passing it directly causes the issue as soon as _getPrototypeOf(Fail).apply(this, arguments) is attempted, since you usually cannot .apply(...) classes, or at least you surely cannot do that with native DOM classes (that also require to be register as custom elements to be used as such).

A possible solution could be to test at runtime if a class is native, and in that case enforce the usage of _wrapNativeSuper.

However, since this issue is very DOM use cases specific, maybe forcing _wrapNativeSuper for any class that is either explicitly native or instanceof Element could work?

Yet I think the easiest way to go would be to pass _isNativeFunction(Class) ? _wrapNativeSuper(Class) : Class when it's not instantly possible to recognize native from user-land.

Additional context/Screenshots
Screenshot from 2019-06-06 09-50-25

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Jun 6, 2019

Hey @WebReflection! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@WebReflection

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

To whom it might concern, the way I've solved this issue in user land is through an extend.js file that exports a function after feature checking at runtime if the code has been transpiled down to ES5 or not.

const {construct, setPrototypeOf} = Reflect;

let transpiled = false;

try {
  // the angry koala check https://twitter.com/WebReflection/status/1133757401482584064
  transpiled = !!new {o(){}}.o;
} catch($) {}

export default transpiled ?
  function (Super) {
    const Class = function () {
      return construct(Super, arguments, Class);
    };
    setPrototypeOf(Class, Super);
    setPrototypeOf(Class.prototype, Super.prototype);
    return Class;
  } :
  Super => class extends Super {};

Above code is used in heresy 🔥 to avoid issues when defining components via objects intead of classes.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Since wrapNativeSuper is way slower than just .apply and extending dynamically retrived dom constructors isn't a common case, would it be ok to introduce a safeExtendCheck option to @babel/plugin-transform-classes to always use _isNativeFunction(Class) ? _wrapNativeSuper(Class) : Class?

@WebReflection

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

would it be ok to introduce a safeExtendCheck option to @babel/plugin-transform-classes to always use _isNativeFunction(Class) ? _wrapNativeSuper(Class) : Class?

That would work, but I'd still need to use defensive code and runtime transpiled checks to be sure whoever imported my module and mixed with the rest of the code doesn't need to have that flag on.

However, wouldn't it be safe to assume that such check should always be performed when:

  1. the passed class was retrieved form accessing a .constructor
  2. the .constructor was accessed from a DOM node (or after document.createElement)

Specially for custom elements builtins, where using document.createElement is the way to safely initiate components at runtime, I believe we could confine the ternary check only for that case, or for both cases described.

Is this an option? Otherwise having at least a flag to tell people about would be surely better than current state.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.