fix(fromEvent): pass options in unsubscribe

[cartant]

Description:

Fixes the problem reported in #3349. In Chrome, at least, if options are passed in the call to addEventListener, but not in the call to removeEventListener, the listener is not removed.

The PR adds an expectation to an existing test and passes the options in the call to removeEventListener, too.

Related issue (if exists): #3349

[kwonoj]

So actually MDN describes this topic already: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener

While addEventListener() will let you add the same listener more than once for the same type if the options are different, the only option removeEventListener() checks is the capture/useCapture flag. Its value must match for removeEventListener() to match, but the other values don't.

It's worth noting that some browser releases have been inconsistent on this, and unless you have specific reasons otherwise, it's probably wise to use the same values used for the call to addEventListener() when calling removeEventListener().

Change looks legit to me.

[rxjs-bot]

	Messages
📖

CJS: 1313.3KB, global: 698.6KB (gzipped: 114.5KB), min: 135.0KB (gzipped: 29.8KB)

Generated by 🚫 dangerJS

[coveralls]

Coverage remained the same at 97.517% when pulling f1872b0 on cartant:issue-3349 into 1e4ca05 on ReactiveX:master.

[crisbeto]

The options aren't necessarily a boolean, they can be an EventListenerOptions object as well.

[cartant]

I'm unaware of why the boolean assertion was used in the original code's call to the addEventListener method. Perhaps the typings in an earlier version of TypeScript were not representative of the API? The current typings seem okay, so I guess this could be changed. However, it is only a type assertion and has no runtime effect.

[benlesh]

Thanks, @cartant! :)

[cartant]

@benlesh Before I rebased this PR - to accommodate the refactorings in master - it would have be cherry-pickable into stable. Do you see any need for another PR to fix this in stable, too?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.