Dedupe event listeners

[aweary]

Currently, each instead of Perimeter will register a mousemove and resize listener on window. While I don't recommend having tons of these on a single page, it may be better for performance to register a single listener and iterate over each registered instance. So we need to:

• Determine if its more performant to use a single listener (maybe browsers can parallelize multiple listeners better?)
• Find a universal way for each instance to register with a global listener. The easiest solution may be to track a property on the global window but other suggestions welcome

[jnsdls]

Feels like a good place to use a MouseMoveProvider component wrapping the application (like the Provider from react-redux) and providing a subscription method to move events on the context -> the Perimeter component on componentWillMount subscribes to the MouseMoveProvider via the exposed context method and unsubscribes on componentWillUnmount

I feel like that should work?

Sorry for the terrible formatting, I'm on my phone. Happy to submit a PR once I get to work that implements this.

[aweary]

I posted an idea in #6 (comment) on how we might approach this

What if we instead provided an API for this instead of providing a default solution? Instead of always deduping listeners (which might actually be more expensive for 1-2 instances) we add an optional prop so users can pass in their own implementation (so deduping behavior can be app-specific).

[aweary]

@jnsdls I like the MouseMoveProvider idea, very familiar to anyone who uses Redux and the like. Then we could still support users who don't care about the added complexity of deduping listeners.

[aweary]

@tkh44 moving the conversation from #6 for better visibilty

It sounds like a good idea, but I'm just wondering what the benefit would be compared to the added complexity (from user perspective). It seems like it would add a lot of moving parts to a beautifully simple api.

The idea is that not everyone might want to dedupe listeners, and that for a small handful of listeners it might actually be less performant (if there's 1-5 the cost of iterating over the array and calling each function might be more than having 1-5 individual listeners). I definitely want to profile it to be sure.

It also:

• Keeps lib size ~small by default (not including dedupe logic until its needed)
• Lets uses control dedupe implementation. Adding the listener, removing them, iterating, etc.

Also, If we can show that deduping with any number of instances provides better performance and there's no real demand for a custom event manager API, I'm fine including #6 by default.

[jnsdls]

I agree that it's quite a bit of moving parts if you want to make it conditional and let people pass their own register & remove listeners functions. (I like the customizability aspect but I'm not sure if it's necessary, we should just come up with the 90 percentile best solution and go with that.)

In my proposed solution we could handle the "global vs non-global" problem very simply by the way:

1. check if there is a MouseMoveProvider on the context
2. if yes -> subscribe || if no -> use own window.eventListener

and then if we wanted to go super deep we could still do the eventManager as a potential prop to (think history in React-Router)

EDIT: Also first we should experiment and prove that it's actually beneficial to have 1 global listener rather than a bunch of listeners. (I'm confident it is, but need to test what impact it makes anyways.)

[tkh44]

After thinking about this a while I think the MouseMoveProvider is a better option.

[jnsdls]

I'll take a stab at it and send a PR

[jnsdls]

the more I think about this the more it makes sense to me to extract this into it's own module and make it something more like a

which would work essentially the same way as the proposed but it would work for any event, so in this case for "mousemove" and "resize".

it would expose the component as well as a withEvents() function to generate a HOC that puts the subscribe & unsubscribe methods on the props of a wrapped component. @aweary, @tkh44 do you guys think that makes sense? could be a peer-dependency or an optional dependency react-perimiter.

[jnsdls]

@aweary haven't done a readme yet, but here it is: https://github.com/jnsdls/react-listener-provider I'll make a new PR tomorrow that implements it in Perimiter.

(I cloned your project structure, so have a look in the demo folder if you're curious.)

[jnsdls]

added the option to use react-listener-provider to the README: #15

[jnsdls]

since #14 and #15 are merged now, I think this can be closed?

[aweary]

Yep, thanks everyone!