krakenjs/beaver-logger

Event listeners on `window` breaks server-side rendering

kweiberth opened this issue · 6 comments

kweiberth commented

Hello! 👋

It looks like a recent PR/release (#31) added two window event listeners to the main thread/initialization of this package:

window.addEventListener('beforeunload', () => {
    immediateFlush();
});

window.addEventListener('unload', () => {
    immediateFlush();
});

Unfortunately, this breaks SSR with following error:

ReferenceError: window is not defined
    at Module.Logger (/Users/kweiberth/code/financeportalnodeweb/node_modules/beaver-logger/dist/beaver-logger.js:936:13)

I think possible the easiest/best solution is to wrap these initializers in a condition that checks for window, e.g.

if (typeof window === 'object') {
    // add event listeners
}

What do you think?

kweiberth commented

In the meantime, I was able to lock in at beaver-logger@4.0.15 and avoid this issue.

mstuart commented

@mnicpt can you take a look at ^^?

  • 👍1
bluepnume commented

(it would be good to add a jest test with env:node so we can quickly see if any future calls to window cause the same issue in a non-browser env)

  • 👍1
mnicpt commented

@bluepnume I thought Logger was meant for client-side only. Should @kweiberth be creating a custom logger that he can configure with the express endpoint on the server? We reference the window all over the place in logger.js.

kweiberth commented

@mnicpt I think it is meant for client-side, but with server-side rendering, e.g. with React server-side rendering, the client-side components and code are rendered on the server, i.e. they are rendered to string to be passed down to client as initial HTML markup.

If uses of window or other client-side libraries are contained to lifecycle methods, e.g. on click handlers, React useEffect, etc., then we're okay bc those processes won't execute on the server. But in this specific case, the event listeners are being added to window in the "main thread"/index.js of this package. Thus, when it's being rendered on the server, the code is imported, immediately executes and fails.

  • 👍1
mnicpt commented

@kweiberth This has been merged and will be deployed this week sometime. Thanks.

  • 👍1