Inferno: Declaration in inferno-dom breaks SSR

[nightwolfz]

https://github.com/trueadm/inferno/blob/dev/packages/inferno-dom/src/index.js#L5

ReferenceError: window is not defined

[trueadm]

@nightwolfz I wonder why this is getting pulled in? inferno-dom shouldn't be used on Node.

[nightwolfz]

@trueadm I get the error just from importing inferno-dom even if I don't use the functionality on the server-side.

A simple

if (typeof window !== 'undefined') {
    window.__INFERNO_DEVTOOLS_DOM_ROOTS__ = roots;
}

should fix this.

[buzinas]

What @trueadm said is completely true. Usually when you're server rendering, you have a client folder, a common one, and a server one. InfernoDOM should be used only inside the client (while InfernoServer only inside the server).

I always use server rendering in all my applications and never got this error.

[nightwolfz]

@buzinas I have exactly the same structure. Are you sure you are on the dev branch ?

[trueadm]

@nightwolfz I'm confused here. the inferno-dom bundle file is the only place where this can be created. Why is this being used on your Node stack? The only way to take it in is being doing a require on inferno-dom.

[nightwolfz]

@trueadm

My app is isomorphic/universal. Same component rendered on both server and client, however only the client version executes DOM methods.

Imagine it's this component (it's not) which imports inferno-dom (for findDOMNode) and executes it on componentDidMount.

This same component is also executed on node for SSR and for fetching server-side data with fetchData.

Now with window in inferno-dom I had a few options:

• Create a conditional inferno-dom-conditional.js which decides either to include or not inferno-dom
• Use a require() inside componentDidMount instead of import
• Write a babel plugin to exclude inferno-dom conditionally
• ...Or add an if to revert to previous functionality.

I prefer the last one.

[trueadm]

@nightwolfz I'm still a bit confused: https://github.com/nightwolfz/inferno-starter/blob/master/src/client/components/Todos.js doesn't require inferno-dom either, so it shouldn't be in there. We can however use the utils.ts isBrowser flag to wrap the statement like we have done elsewhere in the codebase but given that this was a bundle file, not actual source I was hoping we'd be able to avoid it.
Update: just re-read your reply. I understand now, although I think findDOMNode is on the list of things to be removed, as it is in React, so I'd avoid using it in favour of refs

If the problem is that inferno-mobx requires inferno-dom, that shouldn't be the case anymore as this package is now in core and can reference the relevant parts of Inferno directly, thus negative the need to have this bit of code in it.

On a side note, it might be easier to make this code global somehow so it's isomorphic on both Node and in the browser as it's meant to be used for a debugging hook.

[buzinas]

@nightwolfz now I got what you mean. You never said you were using findDOMNode in your original post. This explains why I never saw this error: I simply never use this method.

[nightwolfz]

@trueadm It wasn't inferno-mobx that was causing the problem at that moment, I was migrating a dropdown component from React. Although now that you mention it, I'm also using findDOMNode.

I'm in favour of getting rid of that method but you know that many people are going to cry over it.

Either-way I should be able to include it on node and/or electron even if I don't use any of inferno-dom methods...

[trueadm]

@nightwolfz I know, we can always add findDOMNode to the inferno-compat? It's just a lot of code for no real gain.