`prefer-add-event-listener`: Bad autofix when assignment a non-function object

[axetroy]

This is actually in my project, although the code is unreasonable.

var target

- target.ondrag = true
+ target.addEventListener('drag', true)

In addition, if the parameters do not match the types, an exception will be thrown

addEventListener('click', true) 

// Uncaught TypeError: EventTarget.addEventListener: Argument 2 is not an object.

[axetroy]

There will be many false positives in the future. eg.

target.ondrag = 1
target.ondrag = {}
target.ondrag = []
target.ondrag = true : 1 : 2
target.ondrag = void 0
// There are countless situations ...

I recommend only fixing cases where you know these types: function/undefined/null

target.ondrag = () => {}
target.ondrag = function () {}
target.ondrag = null
target.ondrag = undefined

[fregante]

It seems that anything other than function will just clear the listener.

This function can be extended to cover all those:

https://github.com/sindresorhus/eslint-plugin-unicorn/blob/891842d539d0d002119e9c476a050a12d3936a75/rules/prefer-add-event-listener.js#L50

The rule should probably only autofix when the argument type is known. In other case the rule should still highlight and you'll have to rewrite the code manually.

// Before
target.ondrag = getListenerOrFalse()

// After
if (listenOrNot())
    target.addEventListener('drag', listener)
else
    target.removeEventListener('drag', listener)

node.on* properties behave differently from addEventListener so you're always expected to put some thoughts into this change.

target.ondrag = console.log;
target.ondrag = console.warn;

// is NOT equivalent to

target.addEventListener('drag', console.log);
target.addEventListener('drag', console.warn);

[axetroy]

The rule should probably only autofix when the argument type is known. In other case the rule should still highlight and you'll have to rewrite the code manually.

This is true in theory, but even so, false positives are inevitable.

There are too many hacks here, I prefer to remove out from the recommended rules

https://github.com/sindresorhus/eslint-plugin-unicorn/blob/891842d539d0d002119e9c476a050a12d3936a75/rules/prefer-add-event-listener.js#L54-L55

https://github.com/sindresorhus/eslint-plugin-unicorn/blob/891842d539d0d002119e9c476a050a12d3936a75/rules/prefer-add-event-listener.js#L125-L138

[fregante]

The rule still makes sense and it’s still advisable to use .addEventListener(), the problem is just that the solution isn’t always straightforward.

Autofixes are fine as long as there aren’t too many exceptions. Setting true to a ondrag property was never correct to begin with, it only worked because early JavaScript was extremely lax.