activity-detector icon indicating copy to clipboard operation
activity-detector copied to clipboard

Cannot enable Universal Rendering in projects using activity-detector

Open JesperWe opened this issue 6 years ago • 7 comments

activity-detector is only meaningful when running code in a browser, so for a project that uses Universal Rendering (Server Side Rendering) it should not be part of the server side code path.

The problem is that activity-detector tries to unsafely access the document object already during import, which breaks the app before any conditional code paths can be selected. And it is currently not possible to conditionally import modules.

JesperWe avatar May 18 '18 09:05 JesperWe

What about using dynamic import @JesperWe? I mean: activity detector only Works within a document, right? so I think here is not the place to avoid accessing a document.

A different approach could be to Access lazily to the document insterad of at the first load.

SergioMorchon avatar May 18 '18 09:05 SergioMorchon

That's available in some specific environments only. Adding a simple guard for the availability of document around your initialization would make activity-detector more compatible with SSR independently of stage 3 Javascript proposals.

JesperWe avatar May 18 '18 09:05 JesperWe

You could just add this to your package.json, and then conditionally check if the value is undefined/false then don't use it in logic.

"browser": {
  "activity-detector": false
}

Webpack and Browserify both respect this as far as I know.

niftylettuce avatar Jan 28 '19 23:01 niftylettuce

I've had this same issue. We've forked the code (since it's just one file) and moved the document read into the main body of activityDetector. So far it's working fine.

louh avatar Jan 03 '20 02:01 louh

I'm trying to use it with a Sapper project and since Sapper is the SSR belt on top of Svelte I have the exact same issue with SSR since there is no document on the server. @atabel could we make the fix as suggested above so it works with SSR tools such as Sapper?

I've just tried @louh 's suggestion, moving the check for document inside activityDetector like so:

var activityDetector = function activityDetector () {


    // moved below snippet inside activityDetector so it works with SSR frameworks such as Sapper
    if (typeof document.hidden !== 'undefined') {
      hidden = 'hidden';
      visibilityChangeEvent = 'visibilitychange';
    } else {
        var prefixes = ['webkit', 'moz', 'ms'];
        for (var i = 0; i < prefixes.length; i++) {
            var prefix = prefixes[i];
            if (typeof document[prefix + 'Hidden'] !== 'undefined') {
                hidden = prefix + 'Hidden';
                visibilityChangeEvent = prefix + 'visibilitychange';
                break;
            }
        }
    }

    var _listeners;

    var _ref = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : {},
        _ref$activityEvents = _ref.activityEvents,
        activityEvents = _ref$activityEvents === undefined ? DEFAULT_ACTIVITY_EVENTS : _ref$activityEvents,
        _ref$inactivityEvents = _ref.inactivityEvents,
        inactivityEvents = _ref$inactivityEvents === undefined ? DEFAULT_INACTIVITY_EVENTS : _ref$inactivityEvents,
        _ref$ignoredEventsWhe = _ref.ignoredEventsWhenIdle,
        ignoredEventsWhenIdle = _ref$ignoredEventsWhe === undefined ? DEFAULT_IGNORED_EVENTS_WHEN_IDLE : _ref$ignoredEventsWhe,
        _ref$timeToIdle = _ref.timeToIdle,
        timeToIdle = _ref$timeToIdle === undefined ? 30000 : _ref$timeToIdle,
        _ref$initialState = _ref.initialState,
        initialState = _ref$initialState === undefined ? DEFAULT_INITIAL_STATE : _ref$initialState,
        _ref$autoInit = _ref.autoInit,
        autoInit = _ref$autoInit === undefined ? true : _ref$autoInit;


...

It all works fine on Sapper with SSR now too. Can we please make this change and make it offical so all SSR frameworks can us activity-detector as well! 🥳

evdama avatar Jul 24 '20 07:07 evdama

@evdama That's exactly what we did. And we have our project running in production quite successfully this way, so I think it's a worthy change to make. Thanks for separately trying out and confirming that this works.

I'm going to create a PR for this to nudge this along.

louh avatar Dec 22 '20 23:12 louh

@atabel Please take a look, thank you!

louh avatar Dec 22 '20 23:12 louh