novuhq / novu

Open-Source Notification Platform. Embeddable Notification Center, E-mail, Push and Slack Integrations.
https://novu.co
Other
34.37k stars 3.5k forks source link

🚀 Feature: Better Client Pagination Control #3589

Open david-morris opened 1 year ago

david-morris commented 1 year ago

🔖 Feature description

Headless/React hooks client should allow:

🎤 Why is this feature needed ?

In my use-case, messages are associated with an object external to Novu and can be searched by that object's details. This means that client-side filtering needs to occur and I can't know ahead of time which notifications are relevant to what the user is trying to do.

This feature would also drastically speed up notification centers with a different number of items. For example, the Github notification center shows around 22 items per page. Currently, a view like that would require 3 roundtrips and thinking about timelines of fetching in a not very React way.

✌️ How do you aim to achieve this?

🔄️ Additional Information

It appears that fetchNotifications only fetches the first page of notifications? That violates the principle of least astonishment since there is no other control over pagination/loading and should probably be explicitly stated in the documentation. I expected the "get me out of having to worry about fetching" setting to fetch all pages, not just the first one.

Hmm, it would be nice if

👀 Have you spent some time to check if this feature request has been raised before?

🏢 Have you read the Code of Conduct?

Are you willing to submit PR?

None

LetItRock commented 1 year ago

hey @david-morris 👋

Do I understand correctly that your request is about removing the pagination from the fetchNotifications? If yes, unfortunately, I don't think that we will do that because of many reasons: increased response time, performance degradation, resource exhaustion, misuse, abuse, etc.

From what I see the endpoint behind the fetchNotifications allows to control the page and the size, but I see it is missing in the query?: IStoreQuery interface of the fetchNotifications function, but you can try to trick the TS and include there the page, limit options, but the max value for the limit is 100. Unfortunately, I'm not sure if this will help you to solve your problem.

david-morris commented 1 year ago

Hi @LetItRock , I'm using useNotifications rather than fetchNotifications, but page gets ignored and limit will only cause slow, unnecessary fetches. I want to do one roundtrip and get enough notifications that my users are unlikely to run out of cache by searching.

I get not wanting to allow massive fetches, but the current pagination architecture and some of the limitations of the provider puts a giant question mark over my use case.

I'd love to see caching solutions such as editing the cache rather than refetching an entire feed on a delete, or having a way of simultaneously using multiple feeds so that I can use just one provider with just one cache.