clay icon indicating copy to clipboard operation
clay copied to clipboard

Question: ClayDataProvider - Prevent refetch on variable change

Open thektan opened this issue 6 years ago • 10 comments

What is your question?

1. Is there a proper way of using variables without always refetching?

For example:

const {resource, refetch} = useResource({
	link: fetchDocumentsSearchUrl,
	variables: {
		keywords: searchQuery
	}
});

where searchQuery is an input field value.

I'd only want to perform a fetch when a user explicitly clicks a button or enter rather than on input change.

Delays can be set by fetchDelay, but I was curious if there was an option for completely preventing the fetch or if I'd have to build the searchParams into the link.

2. Is there a way to prevent the initial fetch when the component mounts?

Thanks!

thektan avatar Sep 12 '19 19:09 thektan

Also as a sidenote: I noticed this warning message in the source code, but I never saw it because variables would be blank when passing all search params as part of link.

https://github.com/liferay/clay/blob/master/packages/clay-data-provider/src/useResource.ts#L158-L161

Should it be moved before the first return uri.toString();?

thektan avatar Sep 12 '19 19:09 thektan

hey @thektan, answering your questions:

  1. Is there a proper way of using variables without always refetching?

There is currently no way to disable variables refetching, so maybe we can implement an API that allows this...

  1. Is there a way to prevent the initial fetch when the component mounts?

A good question, could you talk about your use case? At the moment there is no way. Every time the component is assembled it will make the request. Maybe we can implement an API to enable this, something like this:

const [getDocs, {resource, refetch}] = useResource({
	lazy: true,
	link: 'https://clay.dev',
	variables: {
		keywords: query,
	},
});

//...
getDocs()

Once lazy is true, the return signature will be changed, returning a function and the data... this allows we not to take away the essence of the refetch method.

Also as a sidenote: I noticed this warning message in the source code, but I never saw it because variables would be blank when passing all search params as part of link.

Oh sorry for that, you're right we should move and improve warning validation, thanks for that.

matuzalemsteles avatar Sep 13 '19 02:09 matuzalemsteles

Once lazy is true, the return signature will be changed

In general, having a return signature that changes based on an option sounds a bit dirty, and probably a bit unpleasant to represent in the type system too.

wincent avatar Sep 13 '19 06:09 wincent

In general, having a return signature that changes based on an option sounds a bit dirty, and probably a bit unpleasant to represent in the type system too.

Maybe, to represent in type system I do not see so many problems, I can cause an overload and that solves... I find something common at least...

type TData = {refetch: () => void, resource: any};

function useResource(props: IResource): TData;
function useResource(props: IResource): [() => void, TData];

function useResource({
//...

Probably the most recommending for this here would be a useLazyResource that has this signature only... but not sure yet, what do you think?

matuzalemsteles avatar Sep 13 '19 13:09 matuzalemsteles

@matuzalemsteles Here's my use case:

I have a modal that initially is blank because there is no search query yet.

Kapture 2019-09-13 at 9 46 39

I think it would appropriate if the return signature was the same, but resource would stay undefined with onNetworkStatusChange to not be called yet. To display some sort of "initial" state, I was attempting to implement a notAsked state:

	const {resource, refetch} = useResource({
		link: fetchDocumentsSearchUrl,
		onNetworkStatusChange: status =>
			setResourceState({
				error: status === 5,
				loading: status < 4,
				notAsked: false // <-- notAsked is default true, but is set false here
			})
	});

thektan avatar Sep 13 '19 16:09 thektan

Thanks @thektan, I think in order not to change the signature, we would have to use refetch as a trigger to get the first request and have the same behavior without lazy but it is a little counterintuitive for me to use refetch for such.

matuzalemsteles avatar Sep 16 '19 17:09 matuzalemsteles

@matuzalemsteles do you know if there's anything more to be done here? I'm currently going through our open issues and it seems a lot have been left in "limbo" - in those cases if we aren't going to do anything I'd suggest closing the issue.

julien avatar Dec 13 '21 13:12 julien

Yeah, I think last year we had decided to move the DataProvider to DXP but we haven't done it yet, we just have one epic to deal with moving the component and replacing all Clay uses with the DXP implementation but we haven't done that yet, the resources of improvement would be in another epic but it has not yet been created.

matuzalemsteles avatar Dec 13 '21 15:12 matuzalemsteles

@matuzalemsteles OK thanks for making in clear. I'll try to move it in the near future (i.e. before 2022 if possible).

julien avatar Dec 13 '21 16:12 julien

LPS Ticket https://liferay.atlassian.net/browse/LPS-190550

matuzalemsteles avatar Jul 13 '23 23:07 matuzalemsteles