Prototype inheritance bug

[PetarMax]

Current behavior

If, when using the .on function, the event type equals any of the properties of Object.prototype (in particular, constructor, hasOwnProperty, isPrototypeOf, propertyIsEnumerable, toLocaleString, toString, valueOf), a JavaScript native TypeError is thrown by the framework.

This is because of how eventsFocus and eventsHover are defined and used. The getEventNameBubbling function returns eventsHover[name] if this is not a falsey value. However, when name === "valueOf", for example, this property is found in Object.prototype and the corresponding function object is returned. Later, a .split will be invoked on that object, assuming incorrectly that it is a string, resulting in the error.

This is a common error when using objects as maps in JavaScript, and TypeScript types are not strong enough to catch it.

Expected behavior

No TypeError should be thrown, eventsFocus[name] and eventsHover[name] should return undefined, and the execution should continue.

Solution

Create eventsFocus and eventsHover using Object.create(null) to disable any prototype inheritance.

[fabiospampinato]

I don't see why anybody would have an event called "valueOf" but this is a bug nonetheless, I never thought of that, I must have introduced similar bugs many times across the things I maintain 🤔

[PetarMax]

Agreed, this is not very likely to be triggered in this setting. We have found similar bugs in several codebases, including one belonging to a major company. jQuery hasn't fixed it yet, also.

[fabiospampinato]

Fixing this, via Object.setPrototypeOf ( obj, null ), would increase the size of the minified and gzipped library by about 0.55%, and creating those objects without their regular prototype will require about at least double the amount of time in Chrome latest, so I think it's better to label this as a "wontfix", as I think this isn't really a real world bug, like pretty much nobody will ever stumble upon this, and if they are calling their events or custom properties "isOwnProperty" or something I think it's kind of on them.

This is a bug nonetheless though, so I'm also keeping this open so that users can learn about it.

[PetarMax]

I'm sorry, I don't quite follow your reasoning. These are just two objects, created statically. How often are they created and what are the times precisely that are being doubled? They are surely in terms of milliseconds or microseconds, are they not?

[fabiospampinato]

I counted 6 objects affected by the same issue across the codebase, they are only created once.

If I remember correctly my crappy benchmark for this said that instantiating those objects without a prototype would require about 0.03ms more on my machine (but this isn't accounting for potentially greater time required to download and parse a slight bigger library).

Pretty negligible, at least on my machine, but it'd say that and the increased size of the library costs more than what fixing this issue is worth 🤷‍♂

[fabiospampinato]

Actually we may use Sets for this, I think all supported browsers support them natively, I'll investigate this 👍