ronantakizawa/codereview-bench huggingface dataset:
Repository: pmndrs/zustand
File: src/index.ts (TypeScript)

Code before:
import { useEffect, useLayoutEffect, useReducer, useRef } from 'react'
import createImpl, {
  Destroy,
  EqualityChecker,
  GetState,
  SetState,
  State,
  StateCreator,
  StateSelector,
  Subscribe,
  StoreApi,
} from './vanilla'
export * from './vanilla'

// For server-side rendering: https://github.com/react-spring/zustand/pull/34
// Deno support: https://github.com/pmndrs/zustand/issues/347
const isSSR =
  typeof window === 'undefined' ||
  !window.navigator ||
  /ServerSideRendering/.test(window.navigator.userAgent)

const useIsomorphicLayoutEffect = isSSR ? useEffect : useLayoutEffect

export interface UseStore<T extends State> {
  (): T
  <U>(selector: StateSelector<T, U>, equalityFn?: EqualityChecker<U>): U
  setState: SetState<T>
  getState: GetState<T>
  subscribe: Subscribe<T>
  destroy: Destroy
}

export default function create<TState extends State>(
  createState: StateCreator<TState> | StoreApi<TState>
): UseStore<TState> {
  const api: StoreApi<TState> =
    typeof createState === 'function' ? createImpl(createState) : createState

  const useStore: any = <StateSlice>(
    selector: StateSelector<TState, StateSlice> = api.getState as any,
    equalityFn: EqualityChecker<StateSlice> = Object.is

Review comment (suggestion):
```suggestion
// Deno support: https://github.com/pmndrs/zustand/issues/347
// In the case of Deno, we'll check the globalThis for the existance of the Deno object to see if we are server side
const isGlobalThisDeno = globalThis && globalThis.Deno ? true : false;
```

Code after:
import { useEffect, useLayoutEffect, useReducer, useRef } from 'react'
import createImpl, {
  Destroy,
  EqualityChecker,
  GetState,
  SetState,
  State,
  StateCreator,
  StateSelector,
  Subscribe,
  StoreApi,
} from './vanilla'
export * from './vanilla'

// For server-side rendering: https://github.com/react-spring/zustand/pull/34
// Deno support: https://github.com/pmndrs/zustand/issues/347
const isSSR =
  typeof window === 'undefined' ||
  !window.navigator ||
  /ServerSideRendering|^Deno\//.test(window.navigator.userAgent)

const useIsomorphicLayoutEffect = isSSR ? useEffect : useLayoutEffect

export interface UseStore<T extends State> {
  (): T
  <U>(selector: StateSelector<T, U>, equalityFn?: EqualityChecker<U>): U
  setState: SetState<T>
  getState: GetState<T>
  subscribe: Subscribe<T>
  destroy: Destroy
}

export default function create<TState extends State>(
  createState: StateCreator<TState> | StoreApi<TState>
): UseStore<TState> {
  const api: StoreApi<TState> =
    typeof createState === 'function' ? createImpl(createState) : createState

  const useStore: any = <StateSlice>(
    selector: StateSelector<TState, StateSlice> = api.getState as any,
    equalityFn: EqualityChecker<StateSlice> = Object.is
Repository: pmndrs/zustand
File: src/context.ts (TypeScript)

Code before:
import React, { createContext, useContext, useRef } from 'react'
import create, { UseStore } from './index'
import { EqualityChecker, State, StateCreator, StateSelector } from './vanilla'

function createZustand<TState extends State>(initialState?: TState) {
  const ZustandContext = createContext<UseStore<TState> | undefined>(undefined)

  const Provider = ({
    createState,
    children,
  }: {
    createState: StateCreator<TState>
    children: React.ReactNode
  }) => {
    const storeRef = useRef() as React.MutableRefObject<UseStore<TState>>

    if (!storeRef.current) {
      storeRef.current = create(createState)
    }

    return React.createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }


Review comment (suggestion):
```suggestion
import create, { UseStore } from 'zustand'
```

We need to alias for development. eslintrc does it. tsconfig has to be modified. check how [jotai](https://github.com/pmndrs/jotai) does it.

Code after:
import {
  ReactNode,
  createElement,
  createContext as reactCreateContext,
  useContext,
  useRef,
} from 'react'
import { UseStore } from 'zustand'
import { EqualityChecker, State, StateSelector } from './vanilla'

function createContext<TState extends State>(_initialState?: TState) {
  const ZustandContext = reactCreateContext<UseStore<TState> | undefined>(
    undefined
  )

  const Provider = ({
    initialStore,
    children,
  }: {
    initialStore: UseStore<TState>
    children: ReactNode
  }) => {
    const storeRef = useRef<UseStore<TState>>()

    if (!storeRef.current) {
      storeRef.current = initialStore
    }
Repository: pmndrs/zustand
File: src/context.ts (TypeScript)

Code before:
import React, { createContext, useContext, useRef } from 'react'
import create, { UseStore } from './index'
import { EqualityChecker, State, StateCreator, StateSelector } from './vanilla'

function createZustand<TState extends State>(initialState?: TState) {
  const ZustandContext = createContext<UseStore<TState> | undefined>(undefined)

  const Provider = ({
    createState,
    children,
  }: {
    createState: StateCreator<TState>
    children: React.ReactNode
  }) => {
    const storeRef = useRef() as React.MutableRefObject<UseStore<TState>>

    if (!storeRef.current) {
      storeRef.current = create(createState)
    }

    return React.createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useZustand = <StateSlice>(
    selector: StateSelector<TState, StateSlice>,
    eqlFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useStore = useContext(ZustandContext)
    if (!useStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useStore(selector, eqlFn)
  }

Review comment (suggestion):
```suggestion
    const storeRef = useRef<UseStore<TState>>()
```
I prefer to annotate type instead of assertion here and assert type in L23, but it's not a big deal.

Code after:
import {
  ReactNode,
  createElement,
  createContext as reactCreateContext,
  useContext,
  useRef,
} from 'react'
import { UseStore } from 'zustand'
import { EqualityChecker, State, StateSelector } from './vanilla'

function createContext<TState extends State>(_initialState?: TState) {
  const ZustandContext = reactCreateContext<UseStore<TState> | undefined>(
    undefined
  )

  const Provider = ({
    initialStore,
    children,
  }: {
    initialStore: UseStore<TState>
    children: ReactNode
  }) => {
    const storeRef = useRef<UseStore<TState>>()

    if (!storeRef.current) {
      storeRef.current = initialStore
    }

    return createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
Repository: pmndrs/zustand
File: src/context.ts (TypeScript)

Code before:
    )
  }

  const useZustand = <StateSlice>(
    selector: StateSelector<TState, StateSlice>,
    eqlFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useStore = useContext(ZustandContext)
    if (!useStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useStore(selector, eqlFn)
  }

  return {
    Provider,
    useZustand,
  }
}

export default createZustand

export const { Provider, useZustand } = createZustand({})


Review comment (refactor):
```suggestion
```
Let's remove this and provide only one usage. I think it's less confusing.

Code after:
    if (!storeRef.current) {
      storeRef.current = initialStore
    }

    return createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }

  return {
    Provider,
    useStore,
  }
}

export default createContext
Repository: pmndrs/zustand
File: src/context.ts (TypeScript)

Code before:
  createContext as reactCreateContext,
  useContext,
  useRef,
} from 'react'
import create, { UseStore } from 'zustand'
import { EqualityChecker, State, StateCreator, StateSelector } from './vanilla'

function createContext<TState extends State>(initialState?: TState) {
  const ZustandContext = reactCreateContext<UseStore<TState> | undefined>(
    undefined
  )

  const Provider = ({
    createState,
    children,
  }: {
    createState: StateCreator<TState>
    children: React.ReactNode
  }) => {
    const storeRef = useRef<UseStore<TState>>()

    if (!storeRef.current) {
      storeRef.current = create(createState)
    }

    return React.createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      // FIXME: this type assertion seems unnecessary, as useStore does accept undefined.
      // Need to fix useStore()'s types
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }


Review comment (suggestion):
```suggestion
    return createElement(
```

Just a minor thing, but please import it directly.

Code after:
  ReactNode,
  createElement,
  createContext as reactCreateContext,
  useContext,
  useRef,
} from 'react'
import { UseStore } from 'zustand'
import { EqualityChecker, State, StateSelector } from './vanilla'

function createContext<TState extends State>(_initialState?: TState) {
  const ZustandContext = reactCreateContext<UseStore<TState> | undefined>(
    undefined
  )

  const Provider = ({
    initialStore,
    children,
  }: {
    initialStore: UseStore<TState>
    children: ReactNode
  }) => {
    const storeRef = useRef<UseStore<TState>>()

    if (!storeRef.current) {
      storeRef.current = initialStore
    }

    return createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }
Repository: pmndrs/zustand
File: src/context.ts (TypeScript)

Code before:
    const storeRef = useRef<UseStore<TState>>()

    if (!storeRef.current) {
      storeRef.current = create(createState)
    }

    return React.createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      // FIXME: this type assertion seems unnecessary, as useStore does accept undefined.
      // Need to fix useStore()'s types
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }

  return {
    Provider,
    useStore,
  }
}

export default createContext


Review comment (suggestion):
The original useStore type is overloaded, so we'd need to do the same here.
If the type assertion is working correctly, that doesn't look too bad.

Code after:
    children: ReactNode
  }) => {
    const storeRef = useRef<UseStore<TState>>()

    if (!storeRef.current) {
      storeRef.current = initialStore
    }

    return createElement(
      ZustandContext.Provider,
      { value: storeRef.current },
      children
    )
  }

  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }

  return {
    Provider,
    useStore,
  }
}

export default createContext
Repository: pmndrs/zustand
File: tests/context.test.tsx (TypeScript)

Code before:
import { StateCreator } from '../src/index'
import createContext from '../src/context'

const consoleError = console.error
afterEach(() => {
  cleanup()
  console.error = consoleError
})

type CounterState = {
  count: number
  inc: () => void
}

it('creates and uses context store', async () => {
  const { Provider, useStore } = createContext<CounterState>()

  const createParam: StateCreator<CounterState> = (set) => ({
    count: 0,
    inc: () => set((state) => ({ count: state.count + 1 })),
  })

  function Counter() {
    const { count, inc } = useStore()
    React.useEffect(inc, [inc])
    return <div>count: {count}</div>
  }

  const { findByText } = render(
    <Provider createState={createParam}>
      <Counter />
    </Provider>
  )

  await findByText('count: 1')
})

it('throws error when not using provider', async () => {
  console.error = jest.fn()

  class ErrorBoundary extends React.Component<{}, { hasError: boolean }> {
    constructor(props: {}) {
      super(props)
      this.state = { hasError: false }
    }
    static getDerivedStateFromError() {
      return { hasError: true }
    }
    render() {
      return this.state.hasError ? <div>errored</div> : this.props.children
    }

Review comment (suggestion):
but I wonder if this is properly typed.
```suggestion
    return <div>count: {count * 1}</div>
```
This can detect if `count` is inferred as number.

Code after:
import create from '../src/index'
import createContext from '../src/context'

const consoleError = console.error
afterEach(() => {
  cleanup()
  console.error = consoleError
})

type CounterState = {
  count: number
  inc: () => void
}

it('creates and uses context store', async () => {
  const { Provider, useStore } = createContext<CounterState>()

  const store = create<CounterState>((set) => ({
    count: 0,
    inc: () => set((state) => ({ count: state.count + 1 })),
  }))

  function Counter() {
    const { count, inc } = useStore()
    React.useEffect(inc, [inc])
    return <div>count: {count * 1}</div>
  }

  const { findByText } = render(
    <Provider initialStore={store}>
      <Counter />
    </Provider>
  )

  await findByText('count: 1')
})

it('uses context store with selectors', async () => {
  const { Provider, useStore } = createContext<CounterState>()

  const store = create<CounterState>((set) => ({
    count: 0,
    inc: () => set((state) => ({ count: state.count + 1 })),
  }))

  function Counter() {
    const count = useStore((state) => state.count)
    const inc = useStore((state) => state.inc)
    React.useEffect(inc, [inc])
    return <div>count: {count * 1}</div>
  }
Repository: pmndrs/zustand
File: tests/context.test.tsx (TypeScript)

Code before:
})

it('throws error when not using provider', async () => {
  console.error = jest.fn()

  class ErrorBoundary extends React.Component<{}, { hasError: boolean }> {
    constructor(props: {}) {
      super(props)
      this.state = { hasError: false }
    }
    static getDerivedStateFromError() {
      return { hasError: true }
    }
    render() {
      return this.state.hasError ? <div>errored</div> : this.props.children
    }
  }

  const { useStore } = createContext<CounterState>()
  function Component() {
    useStore()
    return <div>no error</div>
  }

  const { findByText } = render(
    <ErrorBoundary>
      <Component />
    </ErrorBoundary>
  )
  await findByText('errored')
})


Review comment (suggestion):
we'd like to have another test with selectors.

```ts
  // like this
  const count = useStore((state) => state.count)
  const inc = useStore((state) => state.inc)
```


Code after:
})

it('uses context store with selectors', async () => {
  const { Provider, useStore } = createContext<CounterState>()

  const store = create<CounterState>((set) => ({
    count: 0,
    inc: () => set((state) => ({ count: state.count + 1 })),
  }))

  function Counter() {
    const count = useStore((state) => state.count)
    const inc = useStore((state) => state.inc)
    React.useEffect(inc, [inc])
    return <div>count: {count * 1}</div>
  }

  const { findByText } = render(
    <Provider initialStore={store}>
      <Counter />
    </Provider>
  )

  await findByText('count: 1')
})

it('throws error when not using provider', async () => {
  console.error = jest.fn()

  class ErrorBoundary extends React.Component<{}, { hasError: boolean }> {
    constructor(props: {}) {
      super(props)
      this.state = { hasError: false }
    }
    static getDerivedStateFromError() {
      return { hasError: true }
    }
    render() {
      return this.state.hasError ? <div>errored</div> : this.props.children
    }
  }

  const { useStore } = createContext<CounterState>()
  function Component() {
    useStore()
    return <div>no error</div>
  }

  const { findByText } = render(
    <ErrorBoundary>
      <Component />
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

Code before:
  try {
    storage = getStorage()
  } catch (e) {
    // prevent error if the storage is not defined (e.g. when server side rendering a page)
  }

  if (!storage) {
    return config(
      (...args) => {
        console.warn(
          `Persist middleware: unable to update ${name}, the given storage is currently unavailable.`
        )
        set(...args)
      },
      get,
      api
    )
  }

  const setItem = async () => {
    const state = { ...get() }

    if (whitelist) {
      Object.keys(state).forEach((key) => {
        const keyTyped = key as keyof S
        !whitelist.includes(keyTyped) && delete state[keyTyped]
      })
    }
    if (blacklist) {
      blacklist.forEach((key) => delete state[key])
    }

    return storage?.setItem(name, await serialize({ state, version }))
  }

  const savedSetState = api.setState

  api.setState = (state, replace) => {
    savedSetState(state, replace)
    void setItem()
  }

  // rehydrate initial state with existing stored state
  ;(async () => {
    const postRehydrationCallback = onRehydrateStorage?.(get()) || undefined

    try {
      const storageValue = await storage.getItem(name)

      if (storageValue) {
        const deserializedStorageValue = await deserialize(storageValue)

Review comment (suggestion):
I would do this. Does it work?
```ts
      ;(Object.keys(state) as (keyof S)[]).forEach((key) => {
        !whitelist.includes(key) && delete state[key]
```

Code after:
  try {
    storage = getStorage()
  } catch (e) {
    // prevent error if the storage is not defined (e.g. when server side rendering a page)
  }

  if (!storage) {
    return config(
      (...args) => {
        console.warn(
          `Persist middleware: unable to update ${name}, the given storage is currently unavailable.`
        )
        set(...args)
      },
      get,
      api
    )
  }

  const setItem = async () => {
    const state = { ...get() }

    if (whitelist) {
      ;(Object.keys(state) as (keyof S)[]).forEach((key) => {
        !whitelist.includes(key) && delete state[key]
      })
    }
    if (blacklist) {
      blacklist.forEach((key) => delete state[key])
    }

    return storage?.setItem(name, await serialize({ state, version }))
  }

  const savedSetState = api.setState

  api.setState = (state, replace) => {
    savedSetState(state, replace)
    void setItem()
  }

  // rehydrate initial state with existing stored state
  ;(async () => {
    const postRehydrationCallback = onRehydrateStorage?.(get()) || undefined

    try {
      const storageValue = await storage.getItem(name)

      if (storageValue) {
        const deserializedStorageValue = await deserialize(storageValue)
Repository: pmndrs/zustand
File: src/context.ts (TypeScript)

Code before:
  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }

  const useStoreApi = () => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore
  }

  return {
    Provider,
    useStore,
    useStoreApi,
  }
}

export default createContext


Review comment (refactor):
Oops, sorry, I should have explained better.
This is exactly the pattern we want to avoid. Like, this is the whole point of this utility.
(If this were possible, we wouldn't need `useStore`, or even this utility.)

I hope something like this would work?
```suggestion
    return {
      getState: useProviderStore.getState,
      setState: useProviderStore.setState,
      subscribe: useProviderStore.subscribe,
    }
```

Code after:
  const useStore = <StateSlice>(
    selector?: StateSelector<TState, StateSlice>,
    equalityFn: EqualityChecker<StateSlice> = Object.is
  ) => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return useProviderStore(
      selector as StateSelector<TState, StateSlice>,
      equalityFn
    )
  }

  const useStoreApi = () => {
    // ZustandContext value is guaranteed to be stable.
    const useProviderStore = useContext(ZustandContext)
    if (!useProviderStore) {
      throw new Error(
        'Seems like you have not used zustand provider as an ancestor.'
      )
    }
    return {
      getState: useProviderStore.getState,
      setState: useProviderStore.setState,
      subscribe: useProviderStore.subscribe,
      destroy: useProviderStore.destroy,
    }
  }

  return {
    Provider,
    useStore,
    useStoreApi,
  }
}

export default createContext