Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AutoSizer height and width changed from type number to number | undefined in 1.0.8 #52

Closed
dturcotte opened this issue Apr 7, 2023 · 5 comments

Comments

@dturcotte
Copy link

I'm using react-virtualized-auto-sizer along with react-window and there was a breaking change in 1.0.8: the height and width values provided by AutoSizer are now number | undefined and can't be passed down directly to FixedSizeList from react-window. The react-window list components expect number | string and won't allow undefined.

Here's a codesandbox showing the issue, you can change the installed version between 1.0.7 and 1.0.8 to see the difference:
https://codesandbox.io/s/weathered-silence-op15io?file=/src/App.tsx

I can default the values to 0 if AutoSizer gives me undefined and it seems to work, is this the recommended solution? It would be ideal if the AutoSizer values matched the list props from react-window but it would be helpful to know the right thing to do if AutoSizer gives an undefined width or height.

Thanks!

@bvaughn
Copy link
Owner

bvaughn commented Apr 7, 2023

This is a duplicate of issue #45

Sorry for the inconvenience, but the new Size type is more accurate type than was there prior– as both width and height params are conditional based on the disableWidth and disableHeight props. (I don't know of a better way to model this with TypeScript. Maybe some kind of generic?)

For now, I'd recommend just casting the type.

@bvaughn bvaughn closed this as completed Apr 7, 2023
@dartess
Copy link

dartess commented Apr 9, 2023

@bvaughn yes, this can be solved with generics and overloads, but TypeScript does not support class overloading, only functions. I tried writing function declarations (if your component were a function), and they work fine - depending on the settings of disableWidth and disableHeight , the correct types for Size. width and Size. height come.

I tried to rewrite the component to a function, but judging by the tests, there are unnecessary calls to some functions (although they look logical, on the contrary, I'm surprised that they are not in the current implementation).

@bvaughn
Copy link
Owner

bvaughn commented Apr 9, 2023

Feel free to share what you're referring to- regarding the functional exploration. Not sure what to make of your comment without any code to reference

@dartess
Copy link

dartess commented Apr 9, 2023

@bvaughn sure: https://github.com/dartess/react-virtualized-auto-sizer/commits/fc
Tests are simply copied (replaced imports)
I tried to rewrite the component without changing the logic, but apparently I did not take into account some difference in the behavior of functional and class components, and the test results are different.

@bvaughn
Copy link
Owner

bvaughn commented Apr 10, 2023

Gotcha.

Migrating to a function component would be nice.

Didn't look too close at this. Looks like your useEffect is missing some dependencies in the array. Should be possible to do a rewrite though, but it would be a backwards breaking change (because of hooks APIs) so it would require a major bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants