ionic-team / stencil-store

Store is a lightweight shared state library by the StencilJS core team. Implements a simple key/value map that efficiently re-renders components when necessary.
https://stenciljs.com/
Other
176 stars 12 forks source link

feat: allow removing `onChange` listeners #439

Open LanceStasinski opened 9 months ago

LanceStasinski commented 9 months ago

Prerequisites

Describe the Feature Request

A change listener can be registered to a store via the store.onChange method, but there is no way to remove that listener. It would be useful to have a clear API to remove a change listener. The best way to currently get this functionality is to supply a listener then to nullify that listener when we no longer want that listener to be used. For example:

interface ToDo {
  id: string
  description: string
}

interface Worker {
  doActions(todos: ToDo[]): void
}

interface WorkerStore {
  workers: Record<Worker['id'], Worker>
}

interface ToDoStore {
  todos: ToDo[]
}

const workerStore createStore<WorkerStore>({ workers: {} })
const todoStore = createStore<ToDoStore>({ todos: [] })

function doWork(todos: ToDo[]) {
  workerStore.state.workers['someId'].doActions(todos)
}

class WorkerStore {
  doWork: ((todos: ToDo[]) => void) | null = null

  initialize(todos: ToDo[]) {
    this.doWork = doWork
    const update = (todos: ToDo[]) => this.doWork?.(todos)
    todoStore.onChange('todos', update)
  }

 disconnectListeners() {
    this.doWork = null
  }
}

It seems like we should be able remove a listener rather than changing the method used in the listener.

Describe the Use Case

There are at least two cases in which I would like to turn off a change listener.

One case involves two or more stores that are related. In the example above, the WorkerStore depends on the ToDoStore. It's currently difficult to make a change to the independent store (e.g. todoStore.state.todos) without making changes or triggering side effects in the dependent store (e.g. workerStore). There may be cases where I want to turn off the listener. For instance, if I call todoStore.reset(), I may not want the workerStore to respond to those changes.

Another case occurs when I register a change handler in a class instance that is garbage collected later on. This seems to cause a memory leak because the listener I registered is no longer available. As an example, I have an Ember application that imports a stencil store. I then define a change handler in a component class or service class. When that component/service class instance is destroyed in the Ember runtime loop, the change listener is still registered to the store but there is no actual reference to the listener function. If there was a method to remove a listener, I could call that method in a component's willDestroy hook. The memory leak here is particularly problematic when running test suites.

Describe Preferred Solution

An API similar to the addEventListener/removeEventListener API could be desirable. Perhaps it would be something like the following:

const workerStore = createStore<WorkerStore>({ workers: {} })
const todoStore = createStore<ToDoStore>({ todos: [] })

function doWork(todos: ToDo[]) {
  workerStore.state.workers['someId'].doActions(todos)
}

// Add onChange listener
todoStore.addListener('todos', doWork) // rename onChange

// Remove listener
todoStore.removeListener('todos', doWork)

Describe Alternatives

The alternative is the workaround I detailed in the issue description.

Related Code

No response

Additional Information

No response

rjborba commented 2 months ago

This is VERY important when we take performance in consideration. +1