techniq / mui-downshift

Thin layer over paypal's downshift to use Material UI visual components
https://techniq.github.io/mui-downshift/
MIT License
92 stars 24 forks source link

Consider dropping react-virtualized #54

Open techniq opened 6 years ago

techniq commented 6 years ago

Currently the use of react-virtualized can hinder the development of new features (example) and typically causes some virtualized-only issues with measurement/caching (example), and leveraging CSS can open up more use cases (position: sticky, etc).

While at some point I would like to investigate supporting react-window for windowing as this is a valid use case to support for large lists, but the use of react-virtualized was initially added due to Material-UI's pre-1.x performance (with it's use of inline styles, etc). This is greatly alleviated with the use of JSS and overall performance improvements in Material-UI 1.x+

I did this for mui-table 2.0.0, and released 1.0.0 as mui-virtualized-table and documented some of the benefits here (mostly specific to a table, but related).

This should greatly simplify and cleanup the code base (getting rid of UNSAFE_componentWillReceiveProps for example).

Opening this issue for others to discuss, object, etc. I plan to release the current react-virtualized based version as mui-downshift-virtualized or similar so you could still continue to use it (or just not upgrade mui-downshift to the next major release).

jozsi commented 6 years ago

I'm all in for react-window, especially after having a hard time testing the multi-select version of mui-downshift due to react-virtualized: it's AutoSizer component always returns 0 for width and height because JSDOM doesn't visually render and can't report on the sizes. In this case, Menu doesn't render and ListItems. I needed to mock the AutoSizer component like this:

// <rootDir>/__mocks__/react-virtualized.js
const React = require('react');
const reactVirtualized = require('react-virtualized');

// JSDOM provides 0 as width and height, react-virtualized won't render
// anything in such cases, so we need to mock these values in AutoSizer
module.exports = {
  ...reactVirtualized,
  AutoSizer: ({ children }) => <div>{children({ height: 500, width: 200 })}</div>,
};
techniq commented 5 years ago

@jozsi You could also consider using puppeteer if you need a more full-fledged testing environment (headless chrome) but mocking Autosizer works in this case (and would be faster I would think).

In @vx/text, we need a full DOM implementation to do property text measuring, and are considering using puppeteer there.

andrewmclagan commented 5 years ago

I think this is a great idea!