gnir-work / react-window-dynamic-list

A naive approach to virtualizing a dynamically sized list
49 stars 9 forks source link

Bundled react, react-dom #3

Closed jamuhl closed 4 years ago

jamuhl commented 4 years ago

I just tried your module and looked promising at first.

First issue I had was that all my items rendered with a height of 57px - so I decided to find out what's wrong in my items rendering. So I started to copy over your index.js and asyncMeasurer.js for easier debugging...

Now things get ugly - I get a nasty error:

Warning: Render methods should be a pure function of props and state; triggering nested component updates from render is not allowed. If necessary, trigger nested updates in componentDidUpdate.

Which comes from calling https://github.com/gnir-work/react-window-dynamic-list/blob/master/src/asyncMeasurer.js#L40 from inside the render function of the DynamicList.

So why does this not happen when directly using your module from npm? Finally found out - you bundle react, react-dom with your module -> therefore also the crazy size https://bundlephobia.com/result?p=react-window-dynamic-list@2.1.0 and the warning here https://github.com/transitive-bullshit/create-react-library#use-with-react-hooks

Currently got no idea how to avoid this issue.

jamuhl commented 4 years ago

Can be closed if you prefer...tried https://github.com/bvaughn/react-window/issues/6#issuecomment-588290264 suggestion with currently seems to do what I need...

gnir-work commented 4 years ago

Firstly thanks for the update! I will check about the bundling of React and React DOM. However it shouldn't create the first bug you described with the height problem.. Did you figure that out? If not it would be create if you could send me a sandbox reproducing the bug :bear:

jamuhl commented 4 years ago

I'm rather sure had todo with my RowElement that got rendered...but not dug deeper. Sorry. Might have a look another time - for now I would say this can be closed...