search

JakeSidSmith / react-reorder

Drag & drop, touch enabled, reorderable / sortable list, React component
https://jakesidsmith.github.io/react-reorder/
MIT License
218 stars 58 forks source link

Fix window undefined on server #68

Closed HriBB closed 8 years ago

JakeSidSmith commented 8 years ago

That is an interesting workaround... XD

Unfortunately I think this will cause some issues if for some reason window and this are undefined, as it'll then try to assign properties to undefined.

I'm still unsure how window can be undefined...

JakeSidSmith commented 8 years ago

Something like this seems a little safer: https://github.com/facebook/react/blob/092f5ae867721bdc386120a14f789d0a64d31916/docs/js/react-dom.js

HriBB commented 8 years ago

Argh my head hurts from all these bundlers and configurations and whatnot :)

Yeah, FB uses this template.

I guess in node, window is undefined when you are in server environment. I can try and fix this by using this template, but I'm not sure how to test it properly. What if we added some tests?

I've got some reading to do ...

JakeSidSmith commented 8 years ago

Tests would be good, but I think I'm going to save that until after the rework. :)

JakeSidSmith commented 8 years ago

Could you try it like this? If it works ok, I'll merge your branch (and update my other repos). :)

https://github.com/JakeSidSmith/canvasimo/blob/bc01196d6510b8634fcde27081fa9337b000e645/lib/index.js#L1019

HriBB commented 8 years ago

Sure. I need to test it first, so it might take a few days. I have done some reading and now I know that my initial workaround will not work :)

HriBB commented 8 years ago

@JakeSidSmith I fixed the export following the Canvasimo example. I built and ran the examples afterwards and it works. I also did a npm link into my project that uses react-reorder and server rendering now works.

I am still not sure about the global export. Before you checked for the existence of React and ReactDOM on the root object, which I don't.

Anyway, please review and let me know if it's OK.

JakeSidSmith commented 8 years ago

The global export should be fine. If React or ReactDOM aren't available that's the developers fault. XD Or that they do not support being used globally.

Looks good to me! ✨