Coder Social home page Coder Social logo

Comments (21)

bvaughn avatar bvaughn commented on May 18, 2024 3

This library will be released as react-window and will not be merged into react-virtualized. Initially, I considered releasing these API changes as version 10 of react-virtualized but decided against it because I don't intend to support as many types of components (like Collection or Masonry) and I don't want to make people choose between upgrading and losing functionality or staying on an old version.

I hope react-virtualized will continue to be maintained and released in parallel with this library, and I think there are some lessons learned here that could improve performance for react-virtualized- but that's it.

Whether you should build on top of this or react-virtualized is up to you. I won't promise to support all of the API surface for react-virtualized here (e.g. onScrollbarPresenceChange). I think the API surface for react-virtualized got too big and hard to maintain and I'd like to avoid that with this library.

Cheers!

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024 1

This statement still does not make sense to me:

is there any performance concern to memoize function(param1, param2) over function({param1, param2}), I think if we use the later form directly there is no need to memoize?

The onScroll callback should only be called if one of its properties change. Since it uses named parameters (aka an inline object), its properties will always appear to "change" from the POV of a memoization library.

So I use a memoization wrapper with regular, non-named parameters– (which is fine, because I'm not worried about getting their order messed up locally)– to control when I call the external prop– (which uses named parameters, for users convenience).

This is nice because it frees me up from having to explicitly track and compare each individual parameter before calling the function.

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024 1

Here it is, BaseTable with frozen rows and columns support and other features, but based on react-virtualized right now, I planed to migrate to use react-window in the next version
I didn't expect it would take so long to open source it (I'm a bit busy with other stuffs)

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

Thanks for you quick response, I also agree we don't need align all the apis from react-virtualized, as I said I can implement onScrollbarPresenceChange on my side.

Then I'm going to use this library directly, thanks so much for those great libraries.

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

Cool! Let me know how it turns out. 😄

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

It turned out that I have to use a modified version, there are several limitations for my scenario:

  1. GridItem/ListItem is over optimized, if the data set changed, the items should be re-rendered, but as Item is a pure component, which will prevent the re-render. What about leave the optimization to the user to inform them using pure component to avoid unnecessary re-render.
  2. provided keys would lead potential problems, if I insert a item in the front of the data set, it will reuse the first rendered item. What about provide an keyGetter to custom keys, that would resolve the above problem too.
  3. I need an easy way to get the estimated total rows height (I added this method to react-virtualized), but now I have to use this.gridRef._scrollingContainer.children[0].clientHeight to get it (or calculate it again on my side)
  4. I'm using the one-column Grid to implement the Table component, which is much more like the List but with horizontal scroll support. I don't want to use Grid as it adds lots of column related logic. As the above problems, I decide to modify the source code of List to satisfy my needs.

I'll let you know when the component is out, then you would have a better understanding on my concerns.

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

Hey 😄 Thanks for the feedback. Here are some thoughts.

  1. GridItem/ListItem is over optimized, if the data set changed, the items should be re-rendered, but as Item is a pure component, which will prevent the re-render. What about leave the optimization to the user to inform them using pure component to avoid unnecessary re-render.

This has long been the case with react-virtualized as well, as the docs mention.

Since neither library (RV or RW) actually takes the "data set" as a prop– changes to the data set would never cause a re-render anyway, unless the item count changed, in which case both Component and PureComponent would re-render.

  1. provided keys would lead potential problems, if I insert a item in the front of the data set, it will reuse the first rendered item. What about provide an keyGetter to custom keys, that would resolve the above problem too.

This is also the same key strategy used by react-virtualized. You probably wouldn't notice the issue with RV though because react-window memoizes much more efficiently.

A custom keyGetter prop would add the overhead of an additional function call for every visible roll for every scroll event (which happens often). Items aren't expected to change position often (or at least not during a performance sensitive time– like during a scroll) so I wonder if an instance method to clear keys might be better? I don't know.

This is something I'll have to think about a bit more. 😄

  1. I need an easy way to get the estimated total rows height (I added this method to react-virtualized), but now I have to use this.gridRef._scrollingContainer.children[0].clientHeight to get it (or calculate it again on my side)

Why do you need this?

Exposing APIs like the one you describe can make it harder to maintain, refactor, and optimize the library. I learned this lesson with RV, but once something has been added to the library, it's kind of there forever. I hope to have a much higher bar for this sort of addition with react-window.

  1. I'm using the one-column Grid to implement the Table component, which is much more like the List but with horizontal scroll support. I don't want to use Grid as it adds lots of column related logic. As the above problems, I decide to modify the source code of List to satisfy my needs.

I believe you could achieve this same thing by tweaking the style parameter passed to your List item renderer to set a width other than 100%. The List should then scroll horizontally and vertically.

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

@bvaughn for the first one, I think you missed my description? I'm talking about the GridItem not the Grid itself, you can see there are no extra props being passed into it, and it's a pure component, so I just can't make the Item re-render, like reorder the data, unless I reset the cached styles

for the second one, I do encounter with some issues because of that with CRUD, if we just remove the GridItem and leave the optimization to the user, we can pass the rowData.id or something else as the key, then there won't be extra function calls

for the third one, I explained the scenario why I need that api in RV, as I want to implement auto height feature with CRUD and expanding, I don't wont to recalculate the total height as it's already been done in the library

the last one, in that way I still can't get scrollLeft from onScroll, I need it to implement frozen columns feature

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

@bvaughn for the first one, I think you missed my description? I'm talking about the GridItem not the Grid itself, you can see there are no extra props being passed into it, and it's a pure component, so I just can't make the Item re-render, like reorder the data, unless I reset the cached styles

Whoops, I did misread your first one. I'll have to give that one some more thought too.

for the third one, I explained the scenario why I need that api in RV, as I want to implement auto height feature with CRUD and expanding, I don't wont to recalculate the total height as it's already been done in the library

Okay. I don't remember every feature request react-virtualized gets. 😄

the last one, in that way I still can't get scrollLeft from onScroll, I need it to implement frozen columns feature

Why would you need scrollLeft for a one-column list/grid? Are you trying to add some sort of scroll-syncing feature?

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

Why would you need scrollLeft for a one-column list/grid? Are you trying to add some sort of scroll-syncing feature?

correct, for example, I have a header and a body(Grid) to compose the table, when I scroll the body horizontally, I need to sync the scrollLeft of header too

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

Gotcha. This is useful input. I'll give it some thought, at least. No promise to make any changes yet. 😄

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

No hurry, I'm working on a modified version to meet my needs now, hope I can finish the migration soon, here is a peek 😄
frozen-rows

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

Cool, cool. I look forward to seeing the changes you make. Maybe some of them can make their way back upstream.

I'd also be curious to know about how the changes impact performance (if at all). 😄 The new React.unstable_Profiler component could offer some insight here, perhaps. I've been using it to compare react-virtualized 9 to a few of the alpha builds.

PS. The demo looks cool! 😄

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

I've given some more thought to your "pure" component comment. I've decided to treat the children prop as a component rather than just a function, and to remove the default PureComponent wrapper I was using before.

This means that function components won't be memoized by default, but that's probably okay. If you want to avoid re-renders for complex items, you can extend PureComponent or implement shouldComponentUpdate yourself.

This change has been published in the new alpha (2).

I have not yet decided about the key thing you mentioned. The short answer is that this will "work" the way you want now, by default– since there is no PureComponent wrapper being injected from my side. But if you want to use memoization and be able to move items around, you'll still have this problem.

You could work around it by implementing your own shouldComponentUpdate method, but that's kind of a hack and I don't really want to encourage custom sCU implementations.

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

Okay, after some consideration– I've decided to add a new itemKey prop as well, to enable custom keys to be provided. This will be an optional property. I've included this in alpha 3.

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

itemKey is good, i'm using dataKey for my table component
UPDATE: you are treating itemKey as a function, but I use it as a string

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

for the item, what's the different between pass a children and itemRenderer, I changed the children into itemRenderer in my version

BTW, is there any performance concern to memoize function(param1, param2) over function({param1, param2}), I think if we use the later form directly there is no need to memoize? I removed memoize as it's always broken after HMR

from react-window.

bvaughn avatar bvaughn commented on May 18, 2024

for the item, what's the different between pass a children and itemRenderer, I changed the children into itemRenderer in my version

Subjective preference. I like using children better because I think it's cleaner and reads better for the function component use case.

BTW, is there any performance concern to memoize function(param1, param2) over function({param1, param2}), I think if we use the later form directly there is no need to memoize? I removed memoize as it's always broken after HMR

I don't really understand what you're asking, sorry.

Typical memoization won't work if you create a new wrapper object each time you call the function.

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

I don't really understand what you're asking, sorry.

image
I'm referring to this block, you simply memoize and proxy function(param1, param2) to function({param1, param2}), I'm wondering if it's really necessary as the params change all the time

from react-window.

nihgwu avatar nihgwu commented on May 18, 2024

Since it uses named parameters (aka an inline object), its properties will always appear to "change" from the POV of a memoization library.

That’s what I expected, thanks.
Now I get it, as onScroll is not called directly from onScroll event, so there are chances the parameters stay the same, I thought the onScroll callback would be invoked with different parameters all the time

from react-window.

sandys avatar sandys commented on May 18, 2024

@nihgwu - would you consider releasing your table component as open source ? There's a lot we can learn from it as an illustrative example in using react-window.

Here's our attempt on the react-virtualized side :
https://github.com/RedCarpetUp/rv-rich-table

from react-window.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.