fix server side rendering bug

[ebrahimiaval]

fix "ReferenceError: window is not defined" error on SSR project.
in server side 'winodw' object does not exist. we can import 'flickity' package when we need to render slider in client and in server generate pure DOM.

[yaodingyd]

This doesn't look right. Besides we already have the check in componentDidMount lifecycle.

[yaodingyd]

NO, This is right. in SSR componentDidMount do not effect JUST 'constructor' and 'render' work.
you must ignore import window dependency plugins to fix 'window undefined' error. in this commit i ignored flickity to fix it. now we can use in both side.

So in SSR componentDidMount is not called. But the only usage of Flickity is in componentDidMount where it initializes. So what good it does to set to undefined since it's never called?

Can you make sure your SSR window reference is in this lib?

[ebrahimiaval]

NO, This is right. in SSR componentDidMount do not effect JUST 'constructor' and 'render' work.

you must ignore import window dependency plugins to fix 'window undefined' error. in this commit i ignored flickity to fix it. now we can use in both side.

please test it on real SSR project to see error.

[yaodingyd]

Yes, I agree with you on that in SSR componentDidMount does not take effect. My point is Flickity is actually called in componentDidMount, so since this lifecycle is never invoked, how come setting it to undefined solved the issue?

If you want me to test on a real SSR project, can you create a minimum repo to reproduce your issue?

[ebrahimiaval]

Yes, I agree with you on that in SSR componentDidMount does not take effect. My point is Flickity is actually called in componentDidMount, so since this lifecycle is never invoked, how come setting it to undefined solved the issue?

If you want me to test on a real SSR project, can you create a minimum repo to reproduce your issue?

yon can clone my RSSR-dev project and insert your flickity sample in to App.js then run 'npm i' and 'npm run dev' to can test it

[fandy]

Can confirm this is happening for me on SSR. Any updates on merging this @yaodingyd ?

[theolampert]

I can reproduce @ebrahimiaval's issue. I've never had a need for the package server side so don't know how it works but have received a few PRs related to it.

Ultimately it would be great if this was handled upstream since it would fix the issue for others too and not just end uses of this library, which is intended to just be a wrapper for using flickity in react. But it doesn't look like anyone wants to pick this up right now metafizzy/flickity#353.

A check like the one proposed might be good enough for now?

[yaodingyd]

I'll take another look tonight

[yaodingyd]

I would say this should be handled by developers in their own build process. We have PR fix other SSR issues from people and SSR have been working for them, given they probably have the global.window check somewhere. That being said, I'm OK if you @theolampert want to merge this.

[theolampert]

@yaodingyd Yeah I think I'm basically in a agreement but a little on the fence. I like the idea of just being able to install it without any friction even if the behaviour is maybe a little unexpected.

[theolampert]

I believe this is covered by #79 now so I'll close this, thank you for your work @ebrahimiaval