dantrain / react-stonecutter

Animated grid layout component for React
http://dantrain.github.io/react-stonecutter
MIT License
1.21k stars 72 forks source link

when add item too frequent, layout got exception #12

Closed stormslowly closed 8 years ago

stormslowly commented 8 years ago

image

the reason is that when imagesloaded callback is called, the DOM still don't have height (0). so when layout try to layout the item it throws the error.

my current solution is using lodash debounce to slow down the speed of add items.

i will fix it by check the height element when imagesloaded

Uncaught Error: Each child must have an "itemHeight" prop or an "itemRect.height" prop.(anonymous function) @ react-stonecutter.js:1298t.default @ react-stonecutter.js:1292t.default.u.default.createClass.doLayout @ react-stonecutter.js:403CSSGrid_componentWillReceiveProps @ react-stonecutter.js:399O.updateComponent @ ReactCompositeComponent.js:601O.receiveComponent @ ReactCompositeComponent.js:534s.receiveComponent @ ReactReconciler.js:131l.updateChildren @ ReactChildReconciler.js:94D.Mixin._reconcilerUpdateChildren @ ReactMultiChild.js:213D.Mixin._updateChildren @ ReactMultiChild.js:315D.Mixin.updateChildren @ ReactMultiChild.js:303v.Mixin._updateDOMChildren @ ReactDOMComponent.js:938v.Mixin.updateComponent @ ReactDOMComponent.js:767v.Mixin.receiveComponent @ ReactDOMComponent.js:723s.receiveComponent @ ReactReconciler.js:131O._updateRenderedComponent @ ReactCompositeComponent.js:737O._performComponentUpdate @ ReactCompositeComponent.js:715O.updateComponent @ ReactCompositeComponent.js:634O.performUpdateIfNecessary @ ReactCompositeComponent.js:548s.performUpdateIfNecessary @ ReactReconciler.js:165u @ ReactUpdates.js:151i.perform @ Transaction.js:138i.perform @ Transaction.js:138p.perform @ ReactUpdates.js:90T @ ReactUpdates.js:173i.closeAll @ Transaction.js:204i.perform @ Transaction.js:151d.batchedUpdates @ ReactDefaultBatchingStrategy.js:63s @ ReactUpdates.js:201o @ ReactUpdateQueue.js:25f.enqueueSetState @ ReactUpdateQueue.js:209o.setState @ ReactComponent.js:64u.default.createClass.updateRects @ react-stonecutter.js:1208r @ index.js:149d @ index.js:196p @ index.js:184
rafaelcorreiapoli commented 8 years ago

It's happening to me too

rafaelcorreiapoli commented 8 years ago

@stormslowly even when I add some debouncing, sometimes it fails... do you have any advices on how to solve this ?

stormslowly commented 8 years ago

@rafaelcorreiapoli ref the PR https://github.com/dantrain/react-stonecutter/pull/13

rafaelcorreiapoli commented 8 years ago

@stormslowly I tried to install from your fork and I have this: npm i --save git+https://github.com/51ulong/react-stonecutter.git

npm ERR! Linux 4.4.0-31-generic
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "i" "--save" "git+https://github.com/51ulong/react-stonecutter.git"
npm ERR! node v6.2.2
npm ERR! npm  v3.9.5
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn

npm ERR! react-stonecutter@0.3.1 postinstall: `gulp build`
npm ERR! spawn ENOENT
npm ERR! 
npm ERR! Failed at the react-stonecutter@0.3.1 postinstall script 'gulp build'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the react-stonecutter package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     gulp build
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs react-stonecutter
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls react-stonecutter
npm ERR! There is likely additional logging output above.
stormslowly commented 8 years ago

@rafaelcorreiapoli install from npm the devdependencies will not be installed, so the build will failed. i don't have good solution now.

in my project, i just clone the repo in another directory , after npm install all its dependencies ( dependencies devdep and peerdep) then gulp build the lib.

or you can build the lib file then publish another npm package , then install the package.

rafaelcorreiapoli commented 8 years ago

@dantrain Please merge this :)

dantrain commented 8 years ago

I've merged this and added a few retries just in case, hope it helps!