fix: typescript strict on to prevent null calls #477
Conversation
| @@ -205,7 +206,7 @@ export function useSnapPoints({ | |||
| const dragDirection = hasDraggedUp ? 1 : -1; // 1 = up, -1 = down | |||
|
|
|||
| // Don't do anything if we swipe upwards while being on the last snap point | |||
| if (dragDirection > 0 && isLastSnapPoint) { | |||
| if (dragDirection > 0 && isLastSnapPoint && snapPoints) { | |||
There was a problem hiding this comment.
its using snapPoints inside
| @@ -60,7 +60,7 @@ export function useSnapPoints({ | |||
| ); | |||
|
|
|||
| const activeSnapPointIndex = React.useMemo( | |||
| () => snapPoints?.findIndex((snapPoint) => snapPoint === activeSnapPoint), | |||
| () => snapPoints?.findIndex((snapPoint) => snapPoint === activeSnapPoint) ?? null, | |||
There was a problem hiding this comment.
prevent undefined checks
| @@ -1005,8 +1011,10 @@ export const Handle = React.forwardRef<HTMLDivElement, HandleProps>(function ( | |||
| // Make sure to clear the timeout id if the user releases the handle before the cancel timeout | |||
| handleCancelInteraction(); | |||
|
|
|||
| if ((!snapPoints || snapPoints.length === 0) && dismissible) { | |||
| closeDrawer(); | |||
| if (!snapPoints || snapPoints.length === 0) { | |||
There was a problem hiding this comment.
right below this condition there is a call to snapPoints.length and overall there is no effect having an empty array (would return on line 1029)... so returning early
src/index.tsx
Outdated
| pointerStartRef.current = null; | ||
| wasBeyondThePointRef.current = false; | ||
| onRelease(event); | ||
| if (event) { |
There was a problem hiding this comment.
this is the actual fix to the opened issue
There was a problem hiding this comment.
I'm not 100% sure on this one, the safest approach would be to handle the null check inside of the onRelease so we can still set isDragging to false, lmk what you prefer.
There was a problem hiding this comment.
Let's go with the safer approach.
There was a problem hiding this comment.
@emilkowalski done, but playwright had a flaky test, I'm investigating if it's a regression: https://github.com/emilkowalski/vaul/actions/runs/11240999977/job/31251582106?pr=477
There was a problem hiding this comment.
looks like intermittent, locally it passes (not always)... maybe related to the animation timeout?
There was a problem hiding this comment.
only flask/intermittent, nothing that I could find, lmk if you want me to check a specific case.
fixes #476
the root cause was about a missing check caused by event possibly null, suggesting to keep
strict: truein tsconfig to help prevent these bugs, included the fixes to keep it on.