-
Notifications
You must be signed in to change notification settings - Fork 7
Server side rendering #11
base: master
Are you sure you want to change the base?
Changes from 3 commits
f807d51
462718b
2a16bce
22d512e
594cfd5
d60a994
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
'use strict'; | ||
|
||
var isBrowser = (typeof window !== 'undefined'); | ||
|
||
var React = require('react'); | ||
var Imager = require('imager.js'); | ||
|
||
var Imager = isBrowser ? require('imager.js') : null; | ||
|
||
/** | ||
* Imager.jsx Component Factory | ||
|
@@ -20,9 +23,9 @@ module.exports = function (config) { | |
|
||
// Where the magic happens. | ||
// A 'blind' Imager instance shared by all the rendered components. | ||
var imgr = new Imager([], imagerConfig); | ||
var imgr = isBrowser ? new Imager([], imagerConfig) : null; | ||
|
||
if (onResize) { | ||
if (isBrowser && onResize) { | ||
Imager.addEvent(window, 'resize', Imager.debounce(function(){ | ||
Imager.applyEach(imagesCache, function(component){ | ||
component.refreshImageWidth(); | ||
|
@@ -34,16 +37,16 @@ module.exports = function (config) { | |
propTypes: { | ||
// mandatory props | ||
src: React.PropTypes.string, | ||
'background-src': React.PropTypes.string, | ||
// optional props | ||
lowSrc: React.PropTypes.string, | ||
alt: React.PropTypes.string, | ||
className: React.PropTypes.string | ||
}, | ||
|
||
componentDidMount: function () { | ||
this.refreshImageWidth(); | ||
|
||
if (onResize) { | ||
if (isBrowser && onResize) { | ||
imagesCache.push(this); | ||
} | ||
}, | ||
|
@@ -54,7 +57,7 @@ module.exports = function (config) { | |
componentWillUnmount: function () { | ||
var self = this; | ||
|
||
if (onResize) { | ||
if (isBrowser && onResize) { | ||
imagesCache = Imager.applyEach(imagesCache, function(component){ | ||
return component === self ? null : component; | ||
}).filter(function(c){ return c; }); | ||
|
@@ -67,9 +70,9 @@ module.exports = function (config) { | |
|
||
getDefaultProps: function () { | ||
return { | ||
className: imgr.className, | ||
src: imgr.gif.src, | ||
alt: imgr.gif.src | ||
className: imgr ? imgr.className : 'image-replace', // add imagers className to server side rendering. It's kind of dirty but fixes Reacts invalid checksum warning for now. | ||
src: imgr ? imgr.gif.src : '', | ||
alt: imgr ? imgr.gif.src : '' // <-- why are you using the src as alt attribute here? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably a typo. |
||
}; | ||
}, | ||
|
||
|
@@ -79,9 +82,39 @@ module.exports = function (config) { | |
} | ||
}, | ||
|
||
getLowSrc: function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This ideally would have been a different PR but let's not make it more complicated :-) |
||
var lowSrc = this.props.src; | ||
|
||
// use the defined low src property | ||
if (this.props.lowSrc) { | ||
lowSrc = this.props.lowSrc; | ||
} | ||
/*// use imagers gif src | ||
else if (imgr) { | ||
lowSrc = imgr.gif.src; | ||
}*/ | ||
// use the smallest width and replace it in the default src property | ||
else { | ||
var smallestWidth = -1; | ||
|
||
if (config.availableWidths && config.availableWidths.length) { | ||
for (var i = 0, l = config.availableWidths.length; i < l; i++) { | ||
var width = config.availableWidths[i]; | ||
if (smallestWidth === -1 || smallestWidth > width) { | ||
smallestWidth = width; | ||
} | ||
} | ||
} | ||
|
||
lowSrc = this.props.src.replace('{width}', smallestWidth); | ||
} | ||
|
||
return lowSrc; | ||
}, | ||
|
||
getImageSrc: function () { | ||
if (!this.state.width) { | ||
return imgr.gif.src; | ||
return this.getLowSrc(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a bit ambiguous why the lowSrc is loaded if the width is not available. Any reason why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The width is currently calculated via the imager module at and updated afterwards. Using |
||
} | ||
|
||
return imgr.changeImageSrcToUseNewImageDimensions(this.props.src, this.state.width).replace('{height}', this.state.height); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather use the
browser
field ofpackage.json
to shim imager (or not).Server-side, the
onResize
would be always falsy, and the verification would be related toimgr.onResize
rather thanonResize
directly.The code remains equal both for the browser and the server, and the behaviour is only driven by what imager.js is supposed to be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Shiming the Imager directly is definitly the cleaner solution.