Repository: tannerlinsley/react-query
File: packages/query-core/src/queriesObserver.ts (TypeScript)

Code before:
      )

      const newObservers = newObserverMatches.map((match) => match.observer)
      const newResult = newObservers.map((observer) =>
        observer.getCurrentResult(),
      )

      const hasIndexChange = newObservers.some(
        (observer, index) => observer !== prevObservers[index],
      )

      if (prevObservers.length === newObservers.length && !hasIndexChange) {
        const resultChanged = newResult.some((result, index) => {
          const prev = this.#result[index]
          return (
            !prev ||
            result.data !== prev.data ||
            result.isPending !== prev.isPending ||
            result.error !== prev.error
          )
        })

        if (resultChanged) {
          this.#result = newResult
          this.#notify()
        }

        return
      }

      this.#observers = newObservers
      this.#result = newResult

      if (!this.hasListeners()) {
        return
      }

      difference(prevObservers, newObservers).forEach((observer) => {
        observer.destroy()
      })

      difference(newObservers, prevObservers).forEach((observer) => {
        observer.subscribe((result) => {
          this.#onUpdate(observer, result)
        })
      })

      this.#notify()
    })
  }


Review comment (refactor):
I don’t really like the early returns here and the duplicated assignment to `#result` as well as calls to `#notify`, makes it kinda hard to read what is going on. I think we’d want something like this:

- first, check for `hasIndexChange`
  - if we have an index change, we want to compute the difference for observers and destroy / re-create them, then assign `this.#observers = newObservers`
- then, we check for `hasResultChange`
  - if that’s the case, we want to assign `this.#result = newResult`, then check for listeners, then call `this.#notify()`.

this flow ties logic better to the conditions: when indices change, we need to re-do observers and assign them; when result changes, we need to re-assign it. If anything changes, we need to `notify` (I’m assuming here that if indices change, we’ll also get a resultChange)

Code after:
      )

      const newObservers = newObserverMatches.map((match) => match.observer)
      const newResult = newObservers.map((observer) =>
        observer.getCurrentResult(),
      )

      const hasIndexChange = newObservers.some(
        (observer, index) => observer !== prevObservers[index],
      )

      const hasResultChange =
        prevObservers.length === newObservers.length && !hasIndexChange
          ? newResult.some((result, index) => {
              const prev = this.#result[index]
              return !prev || !shallowEqualObjects(result, prev)
            })
          : true

      if (!hasIndexChange && !hasResultChange) return

      if (hasIndexChange) {
        this.#observers = newObservers
      }

      this.#result = newResult

      if (!this.hasListeners()) return

      if (hasIndexChange) {
        difference(prevObservers, newObservers).forEach((observer) => {
          observer.destroy()
        })
        difference(newObservers, prevObservers).forEach((observer) => {
          observer.subscribe((result) => {
            this.#onUpdate(observer, result)
          })
        })
      }

      this.#notify()
    })
  }

  getCurrentResult(): Array<QueryObserverResult> {
    return this.#result
  }

  getQueries() {
    return this.#observers.map((observer) => observer.getCurrentQuery())
  }