Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflicting examples in the docs #629

Closed
evandavis opened this issue Oct 26, 2023 · 3 comments
Closed

Conflicting examples in the docs #629

evandavis opened this issue Oct 26, 2023 · 3 comments

Comments

@evandavis
Copy link

The documentation as written creates confusion around when and when not to use memoization. Specifically, the section Balance Selector Usage contains the following code:

// ❌ DO NOT memoize: will always return a consistent reference
const selectTodos = state => state.todos
const selectNestedValue = state => state.some.deeply.nested.field
const selectTodoById = (state, todoId) => state.todos[todoId]

// ❌ DO NOT memoize: deriving data, but will return a consistent result
const selectItemsTotal = state => {
  return state.items.reduce((result, item) => {
    return result + item.total
  }, 0)
}
const selectAllCompleted = state => state.todos.every(todo => todo.completed)

This makes sense to me! However, the Create Selector Overview shows the following examples:

const selectABC = createSelector([selectA, selectB, selectC], (a, b, c) => {
  // do something with a, b, and c, and return a result
  return a + b + c
})
const selectA = state => state.a
const selectB = state => state.b

const selectA1 = createSelector([selectA], a => a.first)

const selectResult = createSelector([selectA1, selectB], (a1, b) => {
  console.log('Output selector running')
  return a1 + b
})

These examples seem to be explicitly discouraged, but you'd only know that by reading the entire doc. Based on the number of real-world examples I've seen of people memoizing simple lookups, I don't think many people are getting all the way to the bottom.

Could the docs be updated so that the examples follow best-practices? (I'm happy to open a PR if that's helpful.) Thanks!!

@evandavis
Copy link
Author

sorry, dupe of #598

@evandavis evandavis closed this as not planned Won't fix, can't repro, duplicate, stale Oct 26, 2023
@markerikson
Copy link
Contributor

Yeah, the Reselect README could definitely use some love, especially with v5 upcoming.

@aryaemami59
Copy link
Contributor

Yeah, the Reselect README could definitely use some love, especially with v5 upcoming.

I'm on it. It's certainly going to look different...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants