ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.05k stars 1.98k forks source link

dynamically parametrized selectors (dynamic range of computeds) #4241

Closed ducin closed 4 months ago

ducin commented 9 months ago

Which @ngrx/* package(s) are relevant/related to the feature request?

signals

Information

TL;DR; Due to computed caching only the most recent computation, it could happen, that unnecessarily the calculations could be repeated over and over again. This proposal is about a way to cache parametrized/multiple (dynamically) computed values

Example Issue

Let's take a look at the following example (from docs):

type BooksState = {
  books: Book[];
  isLoading: boolean;
  filter: { query: string; order: 'asc' | 'desc' };
};

We can create a computed depending on the current state of the filters:

  withComputed(({ books, filter }) => ({
    visibleBooks: computed(() => {
      const phrase = filter.toLowerCase()
      return books.filter(book => {
        return book.title.toLowerCase().includes(phrase)
      })
    }), 
  })),
const books = [
  {title: "abc 1"},
  {title: "xyz 1"},
  {title: "abc 2"},
  {title: "xyz 2"},
  ...
]

An (angular) computed value is cached for the current value of all its dependencies (current value of filter, e.g.query=""andbooks = [...], all works as expected. But when any dependency change, e.g.filter.querychanges, then the entire **visibleBooks` has to be re-computed**. And that could be a heavy operation. And if we keep on switching between two values of filter.query but we have the same books (e.g. fetched once from the API):

then each time the computed gets dirty, has to recompute, even though the books is the same and, essentially, books.filterBy('abc') result never changes.

This proposal addresses more advanced usecases which, IMO, will appear sooner or later.

The idea is to:

Describe any alternatives/workarounds you're currently using

the only alternative I can think of is:

I would be willing to submit a PR to fix this issue

Example API Proposal

This is just an example of the API, I'm not bound to the API in any way. Design decisions:

API: withParametrizedComputed((state, cacheKeyFn, clearCacheMap) => newComputeds

Example:

  withParametrizedComputed(({ books, filter }, ({ filter }) => filter, { filter: false }) => ({
    visibleBooks: computed(() => {
      const phrase = filter.toLowerCase()
      return books.filter(book => {
        return book.title.toLowerCase().includes(phrase)
      })
    }), 
  })),

The underlying ES Map could look like the following:

Of course the API is far from perfect, it's just a starting point for the discussion.

rainerhahnekamp commented 9 months ago

@ducin would have to read through it multiple times to fully understand it. Are you sure there is no way you can fix it with an equals function in the computed? That one's quite powerful.

Could you add a snippet of how such a parameterized computed would look like? That would help a lot

ducin commented 9 months ago

@rainerhahnekamp @markostanimirovic I have significantly updated the description, and provided an example API to better outline the issue.

rainerhahnekamp commented 9 months ago

Thanks for the clarification Tomasz

markostanimirovic commented 4 months ago

Instead of a custom SignalStore feature (withParametrizedComputed), I'd like to have a custom version of the computed function that has a caching mechanism for the previous results. So it can be used within the SignalStore, but also as a standalone API.

The only problem I see is that it cannot have a computed-like signature with implicit dependency tracking:

const filteredUsers = myComputed(() => {
  return users().filter((u) => u.name.includes(query()));
}, { cacheSize: 10 });

because we won't be able to resolve what are dependent signals and apply proper caching.

Should we reintroduce selectSignal with caching functionality then? πŸ˜„ Maybe with a different name: πŸ‘€

const filteredUsers = explicitComputed(users, query, (users, query) => {
  return users.filter((u) => u.name.includes(query));
}, { cacheSize: 10, ttl: 60 * 1_000 }); // time to leave is 0 by default

I'm going to convert this issue into a discussion. Feel free to share your thoughts!