christianalfoni / impact

Reactive React
https://impact-react.dev
MIT License
112 stars 6 forks source link

Evaluating observers #329

Closed christianalfoni closed 1 month ago

christianalfoni commented 1 month ago

Currently Impact ships with an implicit observer. When using the useStore hook it will internally use useObserver:

function App() {
  using appStore = useStore(AppStore)
}

This does make sense, but it is implicit and diverges with the usage inside stores

function AppStore() {
  const globalStore = useStore(GlobalStore)
}

One uses using, the other const. One can be destructured, the other can not. One does not need to observe, the other does.

We need to allow observation without consuming a store as well, as you could pass signals as props on a component. Which would require:

function App() {
  using _ = useObserver()

  const appStore = useStore(AppStore)
}

So I was reflecting on that we should separate store consumption and observation. Now using a store works exactly the same for both components and other stores. The negative part is having this "dangling variable", which is common in other languages, but not JavaScript. But doing this we can allow for observer, Observer and also Observe.

What means Impact would ship with this set of observer tools:

function App() {
  using _ = useObserver()

  const appStore = useStore(AppStore)
}

const App = observer(function App() {
  const appStore = useStore(AppStore)
})

function App() {
  const appStore = useStore(AppStore)

  return <Observer>{() => <div>{appStore.count}</div>}</Observer>
}

function App() {
  const appStore = useStore(AppStore)

  return <div><Observe>{appStore.countSignal}</Observe></div>
}

I do not want to complicate the API, but there is a lot of diversity in how observation should be done and it is more about giving primitives for teams to decide how they want to do it.

I am leaning towards doing this.

christianalfoni commented 1 month ago

Okay, having Observe does not make any sense. You can use Observer the same way.

christianalfoni commented 1 month ago

So this is implemented now, but discovered an other thing. Will create new issue