search

bvaughn / react-virtualized-auto-sizer

Standalone version of the AutoSizer component from react-virtualized
MIT License
613 stars 82 forks source link

Recurrence of old Resizer bug (Width/Height NaN for AutoSizer #150) #78

Closed saperdadsk closed 9 months ago

saperdadsk commented 9 months ago

Been seeing an error periodically when running some tests on our webpage via Cypress:

`NaN` is an invalid value for the `width` css style property

I don't currently have a consistent reproduction case - error only happens when running in the test environment.

I traced the error down to this library, and specifically https://github.com/bvaughn/react-virtualized-auto-sizer/blob/master/src/AutoSizer.ts#L155-L185, and noticed the comment there referencing an old issue on react-virtualized (https://github.com/bvaughn/react-virtualized/issues/150). It looks like the checks added in https://github.com/bvaughn/react-virtualized/commit/33bdde89dd63983ce2c7b724b5e0ab1ee8c4f726 were modified at some point to use null-coalescing, which no longer handles empty string values.

I'll note we're still running on React 17 and Cypress 10.

bvaughn commented 9 months ago

Nice find. Would you be willing to put together a PR that fixes this? (I'm guessing just replacing the ?? with || would do it, but you have a way to verify the fix)

saperdadsk commented 9 months ago

@bvaughn Sure - let me try it out

saperdadsk commented 9 months ago

Opened a PR with a fix, though I can't figure out how to write tests for this (Spent way too long trying) - it looks like the JSDOM version in the repo doesn't support empty strings for padding

bvaughn commented 9 months ago

Fix published in react-virtualized-auto-sizer@1.0.22

❤️ → ☕ givebrian.coffee