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

Code before:
export type State = Record<string | number | symbol, unknown>
export type PartialState<T extends State> =
  | Partial<T>
  | ((state: T) => Partial<T>)
export type StateSelector<T extends State, U> = (state: T) => U
export type EqualityChecker<T> = (state: T, newState: unknown) => boolean
export type StateListener<T> = (state: T) => void
export type StateSliceListener<T> = (
  state: T | null,
  error: Error | null,
  previousState: T | null
) => void
export interface Subscribe<T extends State> {
  (listener: StateListener<T>): () => void
  <StateSlice>(
    listener: StateSliceListener<StateSlice>,
    selector: StateSelector<T, StateSlice>,
    equalityFn?: EqualityChecker<StateSlice>
  ): () => void
}
export type SetState<T extends State> = (
  partial: PartialState<T>,
  replace?: boolean
) => void
export type GetState<T extends State> = () => T
export type Destroy = () => void
export interface StoreApi<T extends State> {
  setState: SetState<T>
  getState: GetState<T>
  subscribe: Subscribe<T>
  destroy: Destroy
}
export type StateCreator<T extends State, CustomSetState = SetState<T>> = (
  set: CustomSetState,
  get: GetState<T>,
  api: StoreApi<T>

Review comment (suggestion):
```suggestion
export type StateSliceListener<T> = (slice: T, previousSlice: T) => void
```

If such a breaking change is allowed, I would prefer this.

It doesn't conflict with #37, because we no longer use this in useStore. So, I assume nobody uses the second argument and only it confuses types.

Code after:
export type State = Record<string | number | symbol, unknown>
export type PartialState<T extends State> =
  | Partial<T>
  | ((state: T) => Partial<T>)
export type StateSelector<T extends State, U> = (state: T) => U
export type EqualityChecker<T> = (state: T, newState: unknown) => boolean
export type StateListener<T> = (state: T, previousState: T) => void
export type StateSliceListener<T> = (slice: T, previousSlice: T) => void
export interface Subscribe<T extends State> {
  (listener: StateListener<T>): () => void
  <StateSlice>(
    listener: StateSliceListener<StateSlice>,
    selector: StateSelector<T, StateSlice>,
    equalityFn?: EqualityChecker<StateSlice>
  ): () => void
}
export type SetState<T extends State> = (
  partial: PartialState<T>,
  replace?: boolean
) => void
export type GetState<T extends State> = () => T
export type Destroy = () => void
export interface StoreApi<T extends State> {
  setState: SetState<T>
  getState: GetState<T>
  subscribe: Subscribe<T>
  destroy: Destroy
}
export type StateCreator<T extends State, CustomSetState = SetState<T>> = (
  set: CustomSetState,
  get: GetState<T>,
  api: StoreApi<T>
) => T

export default function create<TState extends State>(
  createState: StateCreator<TState>
Repository: pmndrs/zustand
File: src/vanilla.ts (TypeScript)

Code before:
export type State = Record<string | number | symbol, unknown>
export type PartialState<T extends State> =
  | Partial<T>
  | ((state: T) => Partial<T>)
export type StateSelector<T extends State, U> = (state: T) => U
export type EqualityChecker<T> = (state: T, newState: unknown) => boolean
export type StateListener<T> = (state: T, previousState: T) => void
export type StateSliceListener<T> = (state: T, previousState: T) => void
export interface Subscribe<T extends State> {
  (listener: StateListener<T>): () => void
  <StateSlice>(
    listener: StateSliceListener<StateSlice>,
    selector: StateSelector<T, StateSlice>,
    equalityFn?: EqualityChecker<StateSlice>
  ): () => void
}
export type SetState<T extends State> = (
  partial: PartialState<T>,
  replace?: boolean
) => void
export type GetState<T extends State> = () => T
export type Destroy = () => void
export interface StoreApi<T extends State> {
  setState: SetState<T>
  getState: GetState<T>
  subscribe: Subscribe<T>
  destroy: Destroy
}
export type StateCreator<T extends State, CustomSetState = SetState<T>> = (
  set: CustomSetState,
  get: GetState<T>,
  api: StoreApi<T>
) => T

Review comment (suggestion):
```suggestion
export type StateSliceListener<T> = (slice: T, previousSlice: T) => void
```

Let's just rename it.

Code after:
export type State = Record<string | number | symbol, unknown>
export type PartialState<T extends State> =
  | Partial<T>
  | ((state: T) => Partial<T>)
export type StateSelector<T extends State, U> = (state: T) => U
export type EqualityChecker<T> = (state: T, newState: unknown) => boolean
export type StateListener<T> = (state: T, previousState: T) => void
export type StateSliceListener<T> = (slice: T, previousSlice: T) => void
export interface Subscribe<T extends State> {
  (listener: StateListener<T>): () => void
  <StateSlice>(
    listener: StateSliceListener<StateSlice>,
    selector: StateSelector<T, StateSlice>,
    equalityFn?: EqualityChecker<StateSlice>
  ): () => void
}
export type SetState<T extends State> = (
  partial: PartialState<T>,
  replace?: boolean
) => void
export type GetState<T extends State> = () => T
export type Destroy = () => void
export interface StoreApi<T extends State> {
  setState: SetState<T>
  getState: GetState<T>
  subscribe: Subscribe<T>
  destroy: Destroy
}
export type StateCreator<T extends State, CustomSetState = SetState<T>> = (
  set: CustomSetState,
  get: GetState<T>,
  api: StoreApi<T>
) => T
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

Code before:
) => (set: SetState<S>, get: GetState<S>, api: StoreApi<S>): S => {
  const {
    name,
    storage = typeof localStorage !== 'undefined'
      ? localStorage
      : {
          getItem: () => null,
          setItem: () => {},
        },
    serialize = JSON.stringify,
    deserialize = JSON.parse,
  } = options || {}
  
  const setItem = async () => storage.setItem(name, await serialize(get()));

  const savedSetState = api.setState;

  api.setState = (state: PartialState<S>, replace?: boolean) => {
    savedSetState(state, replace);
    (async () => await setItem())();
  };

  const state = config(
    (payload) => {
      set(payload);
      (async () => await setItem())();
    },
    get,
    api
  )

  ;(async () => {
    try {
      const storedState = await storage.getItem(name)
      if (storedState) set(await deserialize(storedState))
    } catch (e) {
      console.error(new Error(`Unable to get to stored state in "${name}"`))
    }
  })()

  return state
}


Review comment (suggestion):
Not a full review, but just noticed this and L153 can be:
```suggestion
      setItem()
```

Code after:
) => (set: SetState<S>, get: GetState<S>, api: StoreApi<S>): S => {
  const {
    name,
    storage = typeof localStorage !== 'undefined'
      ? localStorage
      : {
          getItem: () => null,
          setItem: () => {},
        },
    serialize = JSON.stringify,
    deserialize = JSON.parse,
  } = options || {}

  const setItem = async () => storage.setItem(name, await serialize(get()))

  const savedSetState = api.setState

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

  const state = config(
    (payload) => {
      set(payload)
      setItem()
    },
    get,
    api
  )

  ;(async () => {
    try {
      const storedState = await storage.getItem(name)
      if (storedState) set(await deserialize(storedState))
    } catch (e) {
      console.error(new Error(`Unable to get to stored state in "${name}"`))
    }
  })()

  return state
}
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

Code before:
  storage?: StateStorage
  serialize?: (state: S) => string | Promise<string>
  deserialize?: (str: string) => S | Promise<S>
}

export const persist = <S extends State>(
  config: StateCreator<S>,
  options: PersistOptions<S>
) => (set: SetState<S>, get: GetState<S>, api: StoreApi<S>): S => {
  const {
    name,
    storage = typeof localStorage !== 'undefined'
      ? localStorage
      : {
          getItem: () => null,
          setItem: () => {},
        },
    serialize = JSON.stringify,
    deserialize = JSON.parse,
  } = options || {}

  const setItem = async () => storage.setItem(name, await serialize(get()))

  const savedSetState = api.setState

  api.setState = (state: PartialState<S>, replace?: boolean) => {
    savedSetState(state, replace)
    ;(async () => await setItem())()
  }

  const state = config(
    (payload) => {
      set(payload)
      ;(async () => await setItem())()
    },
    get,
    api
  )

  ;(async () => {
    try {
      const storedState = await storage.getItem(name)
      if (storedState) set(await deserialize(storedState))
    } catch (e) {
      console.error(new Error(`Unable to get to stored state in "${name}"`))
    }
  })()

  return state
}


Review comment (suggestion):
The types are redundant
```suggestion
  api.setState = (state, replace) => {
```

Code after:
  storage?: StateStorage
  serialize?: (state: S) => string | Promise<string>
  deserialize?: (str: string) => S | Promise<S>
}

export const persist = <S extends State>(
  config: StateCreator<S>,
  options: PersistOptions<S>
) => (set: SetState<S>, get: GetState<S>, api: StoreApi<S>): S => {
  const {
    name,
    storage = typeof localStorage !== 'undefined'
      ? localStorage
      : {
          getItem: () => null,
          setItem: () => {},
        },
    serialize = JSON.stringify,
    deserialize = JSON.parse,
  } = options || {}

  const setItem = async () => storage.setItem(name, await serialize(get()))

  const savedSetState = api.setState

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

  const state = config(
    (payload) => {
      set(payload)
      setItem()
    },
    get,
    api
  )

  ;(async () => {
    try {
      const storedState = await storage.getItem(name)
      if (storedState) set(await deserialize(storedState))
    } catch (e) {
      console.error(new Error(`Unable to get to stored state in "${name}"`))
    }
  })()

  return state
}
Repository: pmndrs/zustand
File: tests/basic.test.tsx (TypeScript)

Code before:
import React from "react";
import ReactDOM from "react-dom";
import {
  act,
  cleanup,
  fireEvent,
  render
  } from "@testing-library/react";
import create, {
  State,
  StateListener,
  StateSelector,
  PartialState,
  EqualityChecker,
  StateCreator,
  SetState,
  GetState,
  Subscribe,
  Destroy,
  UseStore,
  StoreApi,
} from '../src/index'
// import { devtools, redux } from '../src/middleware'

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

it('creates a store hook and api object', () => {
  let params
  const result = create((...args) => {

Review comment (suggestion):
```suggestion
import React from 'react'
import ReactDOM from 'react-dom'
import { act, cleanup, fireEvent, render } from '@testing-library/react'
```

reverting this.

Code after:
import React from 'react'
import ReactDOM from 'react-dom'
import { act, cleanup, fireEvent, render } from '@testing-library/react'
import create, {
  State,
  StateListener,
  StateSelector,
  PartialState,
  EqualityChecker,
  StateCreator,
  SetState,
  GetState,
  Subscribe,
  Destroy,
  UseStore,
  StoreApi,
} from '../src/index'
// import { devtools, redux } from '../src/middleware'

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

it('creates a store hook and api object', () => {
  let params
  const result = create((...args) => {
    params = args
    return { value: null }
  })
  expect({ params, result }).toMatchInlineSnapshot(`
    Object {
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

Code before:
): StateCreator<PrimaryState & SecondaryState> => (set, get, api) =>
  Object.assign(
    {},
    initialState,
    create(
      set as SetState<PrimaryState>,
      get as GetState<PrimaryState>,
      api as StoreApi<PrimaryState>
    )
  )

type StateStorage = {
  getItem: (name: string) => string | null | Promise<string | null>
  setItem: (name: string, value: string) => void | Promise<void>
}
type PersistOptions<S> = {
  /** Name of the storage (must be unique) */
  name: string
  /**
   * A function returning a storage.
   * The storage must fit `window.localStorage`'s api.
   * For example the storage could be `AsyncStorage` from React Native.
   *
   * @default () => localStorage
   */
  storage?: () => StateStorage
  /**
   * Use a custom serializer.
   * The returned string will be stored in the storage.
   *
   * @default JSON.stringify
   */
  serialize?: (state: S) => string | Promise<string>
  /**
   * Use a custom deserializer.
   * The returned value will be used as the new state.
   *
   * @param str The storage's current value.
   * @default JSON.parse
   */
  deserialize?: (str: string) => S | Promise<S>
  /**
   * Prevent some items from being stored.
   * Nested properties can be accessed like so: `foo.bar.baz`
   */
  blacklist?: (keyof S)[]
  /**
   * Only store the listed properties.
   * Nested properties can be accessed like so: `foo.bar.baz`
   */
  whitelist?: (keyof S)[]

Review comment (suggestion):
```suggestion
  getStorage?: () => StateStorage
```
We could name like that. Not a strong opinion, but more descriptive and less breaking change.

Code after:
): StateCreator<PrimaryState & SecondaryState> => (set, get, api) =>
  Object.assign(
    {},
    initialState,
    create(
      set as SetState<PrimaryState>,
      get as GetState<PrimaryState>,
      api as StoreApi<PrimaryState>
    )
  )

type StateStorage = {
  getItem: (name: string) => string | null | Promise<string | null>
  setItem: (name: string, value: string) => void | Promise<void>
}
type StorageValue<S> = { state: S; version: number }
type PersistOptions<S> = {
  /** Name of the storage (must be unique) */
  name: string
  /**
   * A function returning a storage.
   * The storage must fit `window.localStorage`'s api (or an async version of it).
   * For example the storage could be `AsyncStorage` from React Native.
   *
   * @default () => localStorage
   */
  getStorage?: () => StateStorage
  /**
   * Use a custom serializer.
   * The returned string will be stored in the storage.
   *
   * @default JSON.stringify
   */
  serialize?: (state: StorageValue<S>) => string | Promise<string>
  /**
   * Use a custom deserializer.
   *
   * @param str The storage's current value.
   * @default JSON.parse
   */
  deserialize?: (str: string) => StorageValue<S> | Promise<S>
  /**
   * Prevent some items from being stored.
   */
  blacklist?: (keyof S)[]
  /**
   * Only store the listed properties.
   */
  whitelist?: (keyof S)[]
  /**
   * A function returning another (optional) function.
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

Code before:
  } = options || {}

  let storage: StateStorage | undefined

  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(set, get, api)

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

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

    if (version) {
      state = {
        ...state,
        __version: version,
      }
    }

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

  const savedSetState = api.setState

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

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

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

      if (storedState) {
        const deserializedState = await deserialize(storedState)


Review comment (suggestion):
This is dangerous, it could conflict with the same property name used in the state.
Can we make it safe?

```js
const data = {
  state,
  version,
}
storage.setItem(name, serialize(data))
```

It's another breaking change...

Or something like:
```js
storage.setItem(name, serialize(state))
storage.setItem(`${name}.version`, serialize(version))
```

Code after:
  let storage: StateStorage | undefined

  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(set, get, api)

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

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

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

  const savedSetState = api.setState

  api.setState = (state, replace) => {
    savedSetState(state, replace)
    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)

        // if versions mismatch, clear storage by storing the new initial state
        if (deserializedStorageValue.version !== version) {
          setItem()
        } else {
          set(deserializedStorageValue.state)
        }
      }
    } catch (e) {
      throw new Error(`Unable to get to stored state in "${name}"`)
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

Code before:
    blacklist,
    whitelist,
    onRehydrateStorage,
    version = 0,
  } = options || {}

  let storage: StateStorage | undefined

  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(set, get, api)

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

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

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

  const savedSetState = api.setState

  api.setState = (state, replace) => {
    savedSetState(state, replace)
    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)

        // if versions mismatch, clear storage by storing the new initial state
        if (deserializedStorageValue.version !== version) {
          setItem()

Review comment (suggestion):
Does it need to warn all the time when updating? why not just at the config time?
```js
  if (!storage) {
    console.warn(...)
    return config(get, set, api)
  }
```

Code after:
    blacklist,
    whitelist,
    onRehydrateStorage,
    version = 0,
  } = options || {}

  let storage: StateStorage | undefined

  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) => {
        !whitelist.includes(key) && delete state[key]
      })
    }
    if (blacklist) {
      blacklist.forEach((key) => delete state[key])
    }

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

  const savedSetState = api.setState

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

  // rehydrate initial state with existing stored state
  ;(async () => {
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' ||
  typeof window.navigator === 'undefined' ||
  /ServerSideRendering/.test(window.navigator.userAgent)

const useIsoLayoutEffect = 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
  ) => {
    const [, forceUpdate] = useReducer((c) => c + 1, 0) as [never, () => void]


Review comment (suggestion):
```suggestion
  window.navigator === undefined ||
```
...is the same as: 
```js
  typeof window.navigator === 'undefined' ||
```

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
  ) => {
    const [, forceUpdate] = useReducer((c) => c + 1, 0) as [never, () => void]