google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.42k stars 1.16k forks source link

"new EventTarget()" or "extends EventTarget" throw errors #3143

Open freshp86 opened 6 years ago

freshp86 commented 6 years ago

Closure compiler should allow instantiating or extending the native EventTarget class, see [1]. Also see [2] which depicts that this is already implemented in Chrome.

Unfortunately closure compiler throws an error for both these cases.

const foo = new EventTarget();
class MyTarget extends EventTarget {}
JSC_NOT_A_CONSTRUCTOR: cannot instantiate non-constructor at line 1 character 12
const foo = new EventTarget();
            ^
JSC_CONFLICTING_EXTENDED_TYPE: MyTarget cannot extend this type; constructors can only extend constructors at line 2 character 0
class MyTarget extends EventTarget {}
^

Even if this is too hard to fix in the default JS compiler externs, there should be a command line flags that allows using a different set of externs to achieve this.

See previous PR about this at https://github.com/google/closure-compiler/pull/2988, which never landed.

[1] https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/EventTarget [2] https://www.chromestatus.com/features/5721972856061952

blickly commented 6 years ago

Looking over that PR, it seems like Bradford's suggestion of aliasing the native EventTarget is a workaround:

e.g. https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540language_out%2520ES6%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F**%2520%2540constructor%250A%2520%2520%2520%2520%2540implements%2520%257BEventTarget%257D%2520*%252F%250Avar%2520NativeEventTarget%2520%253D%2520self%255B%2522EventTarget%2522%255D%253B%250A%250Aclass%2520Foo%2520extends%2520NativeEventTarget%250A%257B%250A%257D%250A%250Aalert(new%2520Foo)

I agree it's not as satisfying as updating the default externs, though.

freshp86 commented 6 years ago

The workaround does not seem to work (I had also tried this before filing this bug). Pasted the contents into a file in Chromium that is currently type checked and here are the errors (you can ignore the filenames)

../../chrome/browser/resources/settings/search_engines_page/search_engines_page.js:8: ERROR - property addEventListener on interface EventTarget is not implemented by type Foo
class Foo extends NativeEventTarget {}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

../../chrome/browser/resources/settings/search_engines_page/search_engines_page.js:8: ERROR - property dispatchEvent on interface EventTarget is not implemented by type Foo
class Foo extends NativeEventTarget {}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

../../chrome/browser/resources/settings/search_engines_page/search_engines_page.js:8: ERROR - property removeEventListener on interface EventTarget is not implemented by type Foo
class Foo extends NativeEventTarget {}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Update: Getting similar errors from the debug service, pasting below:

JSC_INTERFACE_METHOD_NOT_IMPLEMENTED: property addEventListener on interface EventTarget is not implemented by type Foo at line 5 character 0
class Foo extends NativeEventTarget
^
JSC_INTERFACE_METHOD_NOT_IMPLEMENTED: property dispatchEvent on interface EventTarget is not implemented by type Foo at line 5 character 0
class Foo extends NativeEventTarget
^
JSC_INTERFACE_METHOD_NOT_IMPLEMENTED: property removeEventListener on interface EventTarget is not implemented by type Foo at line 5 character 0
class Foo extends NativeEventTarget
^
blickly commented 6 years ago

Ah, good point. I wasn't looking at the warnings.

It's possible to run the compiler with your own externs, but that's a lot poorer UX than just getting the default externs set up correctly. If you wanted to push bradford's workaround further, I think you could redefine those missing methods, e.g. https://goo.gl/atQNrQ, but I agree it's getting pretty ugly.

brad4d commented 6 years ago

Created Google internal bug b/119637644

ChadKillingsworth commented 1 year ago

The workaround for this is pretty gross, but the following code does work:

/**
 * @constructor
 * @implements {EventTarget}
 */
const EventTargetImplementation = window['EventTarget'];
/** @override */
EventTargetImplementation.prototype.addEventListener = window['EventTarget'].prototype.addEventListener;
/** @override */
EventTargetImplementation.prototype.removeEventListener = window['EventTarget'].prototype.removeEventListener;
/** @override */
EventTargetImplementation.prototype.dispatchEvent = window['EventTarget'].prototype.dispatchEvent;

class MyClass extends EventTargetImplementation {}
nicopowa commented 1 month ago

Not a constructor warning also pops with this :

const allTheEvents = new EventTarget();

JSDoc implements tag fixed it but I got more warnings with addEventListener.

Ugly fix : use extends instead.

/**
 * @constructor
 * @implements {EventTarget}
 */
var NativeEventTarget = self["EventTarget"];

const allTheEvents = new NativeEventTarget();

allTheEvents.addEventListener(/* */);

allTheEvents.dispatchEvent(new CustomEvent(...

Thanks !