Option to change debounce wait value and turn off failsafe

[niksy]

Would you consider adding options to change debounce wait value and turning off failsafe check?

Since they apply globally to whole in-viewport module, they can’t be added to each instance, so there should be option to change exported values from module, e.g.

const inviewport = require('in-viewport');
inviewport.debounceWait = 300;
inviewport.failsafe = false;

Another option (and one which will cover case of individual listeners), is to modify instances array to contain properties container, debounceWait and failsafe, and use all 3 properties for checking if instance already exists. If it does, use it, otherwise create new instance with listeners bound to values that user provided.

What do you think? I can create PR if this is something you think is nice to have.

[tzi]

Hello @niksy!

Thanks! It's a great idea to allow to disable the failsafe. It could take a lot of resources, and it is not always needed.

But I'm not sure about the solutions you introduced. What do you think about using the options parameter like for the offset feature?

const inviewport = require('in-viewport');
const elem = document.getElementById('myFancyDiv');

inViewport(elem, { debounceWait: 300 }, visible);
// or
inViewport(elem, { failSafe: false }, visible);

Another great think is, adding this option would say adding documentation about the fail safe feature :speak_no_evil:

Cheers, Thomas.

[niksy]

I’ve actually mentioned idea regarding the option, but maybe I wasn’t clear enough so I’ve created PR for this :)

Regarding failsafe option: do you think that turning it off by default would be a good default? You mentioned:

It could take a lot of resources, and it is not always needed.

So maybe if we provide it as option, and bump new package version (major or minor), leave it disabled by default?

Also, I’ve tried writing tests for this, but test suite wasn’t working correctly at the time so I couldn’t do it. Maybe you could look at it?

[tzi]

Thanks a lot for the PR! :tada:

Regarding failsafe option: do you think that turning it off by default would be a good default?

I prefer to keep the failsafe option enable by default. A developer should be able to use this library without reading the all documentation. Disable the failsafe is, for me, some optimization.

Yes, tests is a problem for now :disappointed: We will look at it.

Cheers, Thomas.

[niksy]

@tzi Ah, I see. It’s just that you mentioned that it isn’t always needed and it’s an edge case so wasting resources is probably not ideal. If it’s documented very well developers know where to look and plan accordingly. The same goes for MutationObserver—it isn’t available everywhere but that’s documented and developers know how to get benefits if they polyfill it.