[material-ui][useScrollTrigger] Do nothing if target is null

[vipierozan99]

Currently if options.target is undefined it tried to default to the window.

In my case I have another component that may or may not be mounted at a given time, so the ref to it might be null.

Right now I do:

const isScrolled = useScrollTrigger({
  target: containerRef.current ?? undefined,
  threshold: 1,
});

but that will default to the window, and if its not present it will throw an error. Which is not what I want exactly.

This change just disables the hook if null is explicitly passed.

[mui-bot]

Netlify deploy preview

https://deploy-preview-45441--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 9cb31e2

[ZeeshanTamboli]

@vipierozan99 Can you fix the failing CI? And in what case is window not present?

[vipierozan99]

@vipierozan99 Can you fix the failing CI? And in what case is window not present?

whoops sorry, first time contributing here 😅, this should fix it. Thanks for the quick response!

w.r.t. the window not being present this line seems to imply its possible, maybe in some testing environments?

material-ui/packages/mui-material/src/useScrollTrigger/useScrollTrigger.js

Line 22 in 680496f

const defaultTarget = typeof window !== 'undefined' ? window : null;

My use case is different tho, I want the target to be an element that might not be mounted yet so I want the hook to do nothing in the meantime, does that make sense?

[ZeeshanTamboli]

@vipierozan99 Can you fix the failing CI? And in what case is window not present?

whoops sorry, first time contributing here 😅, this should fix it. Thanks for the quick response!

w.r.t. the window not being present this line seems to imply its possible, maybe in some testing environments?

material-ui/packages/mui-material/src/useScrollTrigger/useScrollTrigger.js

Line 22 in 680496f

const defaultTarget = typeof window !== 'undefined' ? window : null;

My use case is different tho, I want the target to be an element that might not be mounted yet so I want the hook to do nothing in the meantime, does that make sense?

Can you only call the hook when the target element exists? Instead of modifying useScrollTrigger, you could prevent calling the hook when target is null.

Edit: I know hook shouldn't be called conditionally, but maybe you can avoid rendering that component where the hook is called. Basically in your parent component:

if (!containerRef.current) return <ComponentWithoutHookInside />;

else return <ComponentWithHookInside  />

[vipierozan99]

Edit: I know hook shouldn't be called conditionally, but maybe you can avoid rendering that component where the hook is called. Basically in your parent component:

well, yes, I could do that, but I don't think it's a nice DX, now the parent component has to know if a child is using that hook or not. I'm using it to animate a bunch of components on the page depending on the scroll of another element on the page.

Right now I'm patching MUI using pnpm patch, but I thought its a nice change to contribute to MUI :)

It's also not a breaking change because you have to explicitly pass null to get that behaviour.

[ZeeshanTamboli]

Would this work?

Suggested change

	setTrigger(false);
	return undefined;
	return setTrigger(false);