ronantakizawa/codereview-bench huggingface dataset:
Repository: plotly/dash
File: tests/integration/callbacks/test_basic_callback.py (Python)

Code before:
        return np.array([[1, 2, 3], [4, 5, 6]], np.int32)

    dash_duo.start_server(app)

    assert dash_duo.get_logs() == []


def test_cbsc019_callback_running(dash_duo):
    app = Dash(__name__)

    app.layout = html.Div(
        [
            html.Div("off", id="running"),
            html.Button("start", id="start"),
            html.Div(id="output"),
        ]
    )

    @app.callback(
        Output("output", "children"),
        Input("start", "n_clicks"),
        running=[[Output("running", "children"), "on", "off"]],
        prevent_initial_call=True,
    )
    def on_click(_):
        time.sleep(1.0)
        return "done"

    dash_duo.start_server(app)
    dash_duo.find_element("#start").click()
    dash_duo.wait_for_text_to_equal("#running", "on")
    dash_duo.wait_for_text_to_equal("#output", "done")
    dash_duo.wait_for_text_to_equal("#running", "off")


Review comment (suggestion):
Rather than a `sleep` can we do this test with a `Lock`, like in [`cbsc013`](https://github.com/plotly/dash/blob/4822b7b61915ce21bf8c0367492e70489fd4d02b/tests/integration/callbacks/test_basic_callback.py#L593-L612) for example? It's both faster and more robust that way.

Code after:
        return np.array([[1, 2, 3], [4, 5, 6]], np.int32)

    dash_duo.start_server(app)

    assert dash_duo.get_logs() == []


def test_cbsc019_callback_running(dash_duo):
    lock = Lock()
    app = Dash(__name__)

    app.layout = html.Div(
        [
            html.Div("off", id="running"),
            html.Button("start", id="start"),
            html.Div(id="output"),
        ]
    )

    @app.callback(
        Output("output", "children"),
        Input("start", "n_clicks"),
        running=[[Output("running", "children"), html.B("on", id="content"), "off"]],
        prevent_initial_call=True,
    )
    def on_click(_):
        with lock:
            pass
        return "done"

    dash_duo.start_server(app)
    with lock:
        dash_duo.find_element("#start").click()
        dash_duo.wait_for_text_to_equal("#content", "on")

    dash_duo.wait_for_text_to_equal("#output", "done")
    dash_duo.wait_for_text_to_equal("#running", "off")
Repository: plotly/dash
File: dash/development/_py_prop_typing.py (Python)

Code before:


def get_custom_props(module_name):
    return _get_custom(module_name, "custom_props", {})


def get_custom_ignore(module_name):
    return _get_custom(module_name, "ignore_props", ["style"])


def _clean_key(key):
    k = ""
    for ch in key:
        if ch not in string.ascii_letters + "_":
            k += "_"
        else:
            k += ch
    return k


def generate_any(*_):
    return "typing.Any"


def pascal_case(name: str):
    return name[0].upper() + re.sub(
        r"[\-_\.\s]([a-z])", lambda match: match.group(1).upper(), name[1:]
    )


def generate_shape(type_info, component_name: str, prop_name: str):
    props = []
    name = pascal_case(prop_name)

    for prop_key, prop_type in type_info["value"].items():
        typed = get_prop_typing(
            prop_type["name"], component_name, f"{prop_name}_{prop_key}", prop_type
        )
        if not prop_type.get("required"):
            props.append(f'        "{prop_key}": NotRequired[{typed}]')
        else:
            props.append(f'        "{prop_key}": {typed}')

    shapes.setdefault(component_name, {})
    shapes[component_name][name] = textwrap.indent(
        shape_template.format(
            name=name, values="    {\n" + ",\n".join(props) + "\n    }"
        ),
        "    ",
    )


Review comment (suggestion):
@T4rk1n Is this function covered by tests? Will anything break in a very deep way if this function returns a slightly different output following this change? (The regex looks solid but just in case)

Code after:
    return _get_custom(module_name, "custom_imports", {})


def get_custom_props(module_name):
    return _get_custom(module_name, "custom_props", {})


def get_custom_ignore(module_name):
    return _get_custom(module_name, "ignore_props", ["style"])


def _clean_key(key):
    k = ""
    for ch in key:
        if ch not in string.ascii_letters + "_":
            k += "_"
        else:
            k += ch
    return k


def generate_any(*_):
    return "typing.Any"


def generate_shape(type_info, component_name: str, prop_name: str):
    props = []
    name = pascal_case(prop_name)

    for prop_key, prop_type in type_info["value"].items():
        typed = get_prop_typing(
            prop_type["name"], component_name, f"{prop_name}_{prop_key}", prop_type
        )
        if not prop_type.get("required"):
            props.append(f'        "{prop_key}": NotRequired[{typed}]')
        else:
            props.append(f'        "{prop_key}": {typed}')

    shapes.setdefault(component_name, {})
    shapes[component_name][name] = textwrap.indent(
        shape_template.format(
            name=name, values="    {\n" + ",\n".join(props) + "\n    }"
        ),
        "    ",
    )

    return f'"{name}"'


def generate_union(type_info, component_name: str, prop_name: str):
    types = []
Repository: plotly/dash
File: components/dash-core-components/src/utils/ResizeDetector.js (JavaScript)

Code before:
import React, {createRef, useEffect, useCallback, useMemo} from 'react';
import PropTypes from 'prop-types';

// Debounce 50 ms
const DELAY = 50;

let resizeTimeout;

const ResizeDetector = props => {
    const {onResize, children, targets} = props;
    const ref = createRef();

    const debouncedResizeHandler = useCallback(() => {
        if (resizeTimeout) {
            clearTimeout(resizeTimeout);
        }
        var resizeTimeout = setTimeout(() => {
            onResize(true); // Force on resize.
        }, DELAY);
    }, [onResize]);

    const observer = useMemo(
        () => new ResizeObserver(debouncedResizeHandler),
        [onResize]
    );

    useEffect(() => {
        if (!ref.current) {
            return () => {};
        }
        targets.forEach(target => observer.observe(target.current));
        observer.observe(ref.current);
        return () => {
            observer.disconnect();
        };
    }, [ref.current]);

    return <div ref={ref}>{children}</div>;
};

ResizeDetector.propTypes = {
    onResize: PropTypes.func,

Review comment (suggestion):
Should be replaced with `let` and remove the global one on line 7.

Code after:
import React, {createRef, useEffect, useCallback, useMemo} from 'react';
import PropTypes from 'prop-types';

// Debounce 50 ms
const DELAY = 50;

const ResizeDetector = props => {
    const {onResize, children, targets} = props;
    const ref = createRef();
    let resizeTimeout;

    const debouncedResizeHandler = useCallback(() => {
        if (resizeTimeout) {
            clearTimeout(resizeTimeout);
        }
        resizeTimeout = setTimeout(() => {
            onResize(true); // Force on resize.
        }, DELAY);
    }, [onResize]);

    const observer = useMemo(
        () => new ResizeObserver(debouncedResizeHandler),
        [onResize]
    );

    useEffect(() => {
        if (!ref.current) {
            return () => {};
        }
        targets.forEach(target => observer.observe(target.current));
        observer.observe(ref.current);
        return () => {
            observer.disconnect();
        };
    }, [ref.current]);

    return <div ref={ref}>{children}</div>;
};

ResizeDetector.propTypes = {
    onResize: PropTypes.func,
    children: PropTypes.node,
Repository: plotly/dash
File: components/dash-core-components/src/components/Clipboard.react.js (JavaScript)

Code before:
                {content}
            </LoadingElement>
        ) : null;
    }
}

Clipboard.defaultProps = {
    content: null,
    html_content: null,
    target_id: null,
    n_clicks: 0,
};

Clipboard.propTypes = {
    /**
     * The ID used to identify this component.
     */
    id: PropTypes.string,

    /**
     * The children of this component.  By default copy icon
     */
    children: PropTypes.node,

    /**
     * The children of this component displayed while copying.  By default checked icon
     */
    copied_children: PropTypes.node,

    /**
     * The id of target component containing text to copy to the clipboard.
     * The inner text of the `children` prop will be copied to the clipboard.  If none, then the text from the
     *  `value` prop will be copied.
     */
    target_id: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),

    /**
     * The text to be copied to the clipboard if the `target_id` is None.
     */
    content: PropTypes.string,

    /**
     * The number of times copy button was clicked
     */
    n_clicks: PropTypes.number,

    /**
     * The clipboard html text be copied to the clipboard if the `target_id` is None.
     */
    html_content: PropTypes.string,


Review comment (suggestion):
```suggestion
     * The children of this component displayed after copying.  By default checked icon
```

Code after:
    html_content: null,
    target_id: null,
    n_clicks: 0,
};

Clipboard.propTypes = {
    /**
     * The ID used to identify this component.
     */
    id: PropTypes.string,

    /**
     * Children rendered inside the Clipboard button before copying. By default, a copy icon.
     */
    children: PropTypes.node,

    /**
     * Children rendered inside the Clipboard button after the value has been copied. By default, a check icon.
     */
    copied_children: PropTypes.node,

    /**
     * The id of target component containing text to copy to the clipboard.
     * The inner text of the `children` prop will be copied to the clipboard.  If none, then the text from the
     *  `value` prop will be copied.
     */
    target_id: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),

    /**
     * The text to be copied to the clipboard if the `target_id` is None.
     */
    content: PropTypes.string,

    /**
     * The number of times Clipboard button was clicked
     */
    n_clicks: PropTypes.number,

    /**
     * The clipboard html text be copied to the clipboard if the `target_id` is None.
     */
    html_content: PropTypes.string,

    /**
     * The text shown as a tooltip when hovering over the copy icon.
     */
    title: PropTypes.string,

    /**
     * The icon's styles
     */
Repository: pmndrs/zustand
File: src/index.ts (TypeScript)

Code before:
import { useCallback, useLayoutEffect, useReducer, useRef } from 'react'
import shallowEqual from './shallowEqual'

export type StateListener<T> = (state: T) => void
export type StateSelector<T, U> = (state: T) => U
export type PartialState<T> = Partial<T> | ((state: T) => Partial<T>)

export type State = Record<string, any>
export type SetState<T> = (partialState: PartialState<T>) => void
export type GetState<T> = () => T

export type UseStore<T> = <U = T>(selector?: StateSelector<T, U>) => U

export interface StoreApi<T> {
  getState: GetState<T>
  setState: SetState<T>
  subscribe: (listener: StateListener<T>) => () => void
  destroy: () => void
}

const reducer = <T>(state: any, newState: T) => newState

export default function create<
  TState extends State,
  TSetState extends SetState<TState> = SetState<TState>,
  TGetState extends GetState<TState> = GetState<TState>
>(
  createState: (set: TSetState, get: TGetState) => TState
): [UseStore<TState>, StoreApi<TState>] {
  const listeners: Set<StateListener<TState>> = new Set()

  const setState = (partialState: PartialState<TState>) => {
    state = Object.assign(
      {},
      state,
      typeof partialState === 'function' ? partialState(state) : partialState
    )

Review comment (suggestion):
This type definition should be an overloaded function:
```ts
export type UseStore<T> = {
  (): T
  <U>(selector: StateSelector<T, U>, dependencies?: ReadonlyArray<any>): U
}
```

Code after:
import { useCallback, useLayoutEffect, useReducer, useRef } from 'react'
import shallowEqual from './shallowEqual'

export type StateListener<T> = (state: T) => void
export type StateSelector<T, U> = (state: T) => U
export type PartialState<T> = Partial<T> | ((state: T) => Partial<T>)

export type State = Record<string, any>
export type SetState<T> = (partialState: PartialState<T>) => void
export type GetState<T> = () => T

export type UseStore<T> = {
  (): T
  <U>(selector: StateSelector<T, U>, dependencies?: ReadonlyArray<any>): U
}

export interface StoreApi<T> {
  getState: GetState<T>
  setState: SetState<T>
  subscribe: (listener: StateListener<T>) => () => void
  destroy: () => void
}

const reducer = <T>(state: any, newState: T) => newState

export default function create<TState extends State>(
  createState: (set: SetState<State>, get: GetState<State>) => TState
): [UseStore<TState>, StoreApi<TState>] {
  const listeners: Set<StateListener<TState>> = new Set()

  const setState = (partialState: PartialState<TState>) => {
    state = Object.assign(
      {},
      state,
      typeof partialState === 'function' ? partialState(state) : partialState
    )
    listeners.forEach(listener => listener(state))
Repository: pmndrs/zustand
File: src/index.ts (TypeScript)

Code before:
    const stateSliceRef = useRef(stateSlice)

    // Update refs only after view has been updated
    useLayoutEffect(() => {
      selectStateRef.current = selectState
      stateSliceRef.current = stateSlice
    }, [selectState, stateSlice])

    // Subscribe/unsubscribe to the store only on mount/unmount
    useLayoutEffect(() => {
      return subscribe(() => {
        // Use the last selector passed to useStore to get current state slice
        const selectedSlice = selectStateRef.current(state)
        // Shallow compare previous state slice with current and rerender only if changed
        if (!shallowEqual(stateSliceRef.current, selectedSlice))
          dispatch(selectedSlice)
      })
    }, [])

    return stateSlice
  }

  let state = createState(setState as TSetState, getState as TGetState)
  const api = { destroy, getState, setState, subscribe }

  return [useStore, api] as [typeof useStore, typeof api]
}


Review comment (suggestion):
`as [typeof useStore, typeof api]` is not needed because you correctly defined the return type in the function signature.
```diff
- const api = { destroy, getState, setState, subscribe }
-
- return [useStore, api] as [typeof useStore, typeof api]
+ return [useStore, { destroy, getState, setState, subscribe }]
```

Code after:

    // Update refs only after view has been updated
    useLayoutEffect(() => {
      selectStateRef.current = selectState
      stateSliceRef.current = stateSlice
    }, [selectState, stateSlice])

    // Subscribe/unsubscribe to the store only on mount/unmount
    useLayoutEffect(() => {
      return subscribe(() => {
        // Use the last selector passed to useStore to get current state slice
        const selectedSlice = selectStateRef.current(state)
        // Shallow compare previous state slice with current and rerender only if changed
        if (!shallowEqual(stateSliceRef.current, selectedSlice))
          dispatch(selectedSlice)
      })
    }, [])

    return stateSlice
  }

  let state = createState(
    setState as SetState<State>,
    getState as GetState<State>
  )
  const api = { destroy, getState, setState, subscribe }

  return [useStore, api]
}
Repository: pmndrs/zustand
File: src/index.ts (TypeScript)

Code before:
    if (selectState !== selectStateRef.current) stateSlice = selectState(state)

    // Store in ref to enable updating without rerunning subscribe/unsubscribe
    const stateSliceRef = useRef(stateSlice)

    // Update refs only after view has been updated
    useLayoutEffect(() => {
      selectStateRef.current = selectState
      stateSliceRef.current = stateSlice
    }, [selectState, stateSlice])

    // Subscribe/unsubscribe to the store only on mount/unmount
    useLayoutEffect(() => {
      return subscribe(() => {
        // Use the last selector passed to useStore to get current state slice
        const selectedSlice = selectStateRef.current(state)
        // Shallow compare previous state slice with current and rerender only if changed
        if (!shallowEqual(stateSliceRef.current, selectedSlice))
          dispatch(selectedSlice)
      })
    }, [])

    return stateSlice
  }

  let state = createState(setState as TSetState, getState as TGetState)
  const api = { destroy, getState, setState, subscribe }

  return [useStore, api] as [typeof useStore, typeof api]
}


Review comment (suggestion):
My proposed changes to the `createState` type will require this change:
```ts
let state = createState(
  setState as SetState<State>,
  getState as GetState<State>
)
```

Code after:

    // Store in ref to enable updating without rerunning subscribe/unsubscribe
    const stateSliceRef = useRef(stateSlice)

    // Update refs only after view has been updated
    useLayoutEffect(() => {
      selectStateRef.current = selectState
      stateSliceRef.current = stateSlice
    }, [selectState, stateSlice])

    // Subscribe/unsubscribe to the store only on mount/unmount
    useLayoutEffect(() => {
      return subscribe(() => {
        // Use the last selector passed to useStore to get current state slice
        const selectedSlice = selectStateRef.current(state)
        // Shallow compare previous state slice with current and rerender only if changed
        if (!shallowEqual(stateSliceRef.current, selectedSlice))
          dispatch(selectedSlice)
      })
    }, [])

    return stateSlice
  }

  let state = createState(
    setState as SetState<State>,
    getState as GetState<State>
  )
  const api = { destroy, getState, setState, subscribe }

  return [useStore, api]
}
Repository: pmndrs/zustand
File: src/index.ts (TypeScript)

Code before:
      // isInitial prevents the selector from being called every render.
      isInitial.current && {
        selector,
        equalityFn,
        currentSlice: ((isInitial.current = false), selector(state)),
      }
    ).current as SubscribeOptions<TState, StateSlice>

    // Update state slice if selector has changed or subscriber errored.
    if (selector !== options.selector || options.subscribeError) {
      const newStateSlice = selector(state)
      if (!equalityFn(options.currentSlice as StateSlice, newStateSlice)) {
        options.currentSlice = newStateSlice
      }
    }

    useIsoLayoutEffect(() => {
      options.selector = selector
      options.equalityFn = equalityFn
      options.subscribeError = undefined
    })

    const forceUpdate = useReducer(forceUpdateReducer, 1)[1]
    useIsoLayoutEffect(() => subscribe(forceUpdate, options), [])

    return options.currentSlice as ReturnState<TState, StateSlice>
  }

  const api = { setState, getState, subscribe, destroy }
  state = createState(setState, getState, api)

  return [useStore, api]
}

export { create }


Review comment (suggestion):
The `as {TYPE}` can be removed because all possible return types would be covered in the interface.

Code after:
      isInitial.current && {
        selector,
        equalityFn,
        currentSlice: ((isInitial.current = false), selector(state)),
      }
    ).current as SubscribeOptions<TState, StateSlice>

    // Update state slice if selector has changed or subscriber errored.
    if (selector !== options.selector || options.subscribeError) {
      const newStateSlice = selector(state)
      if (!equalityFn(options.currentSlice as StateSlice, newStateSlice)) {
        options.currentSlice = newStateSlice
      }
    }

    useIsoLayoutEffect(() => {
      options.selector = selector
      options.equalityFn = equalityFn
      options.subscribeError = undefined
    })

    const forceUpdate = useReducer(forceUpdateReducer, 1)[1]
    useIsoLayoutEffect(() => subscribe(forceUpdate, options), [])

    return options.currentSlice
  }

  const api = { setState, getState, subscribe, destroy }
  state = createState(setState, getState, api)

  return [useStore, api]
}

export { create }
Repository: pmndrs/zustand
File: src/middleware.ts (TypeScript)

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

export type StateStorage = {
  getItem: (key: string) => string | null | Promise<string | null>
  setItem: (key: string, value: string) => void | Promise<void>
}
type PersistDefaultOptions<S> = {
  storage: StateStorage
  serialize: (state: S) => string | Promise<string>
  deserialize: (state: string) => S | Promise<S>
}
export type PersistOptions<S> = {
  name: string
} & Partial<PersistDefaultOptions<S>>

export const persist = <S extends State>(
  config: StateCreator<S>,
  options: PersistOptions<S>
) => (set: SetState<S>, get: GetState<S>, api: StoreApi<S>): S => {
  const defaultOptions: PersistDefaultOptions<S> = {
    storage: localStorage,
    serialize: JSON.stringify,
    deserialize: JSON.parse,
  }
  const { name, storage, serialize, deserialize } = {
    ...defaultOptions,
    ...options,
  }

  const state = config(
    (payload) => {
      set(payload)
      ;(async () => {
        storage.setItem(name, await serialize(get()))
      })()

Review comment (suggestion):
```suggestion
  deserialize: (str: string) => S | Promise<S>
```

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

export type StateStorage = {
  getItem: (name: string) => string | null | Promise<string | null>
  setItem: (name: string, value: string) => void | Promise<void>
}
export type PersistOptions<S> = {
  name: string
  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 = localStorage,
    serialize = JSON.stringify,
    deserialize = JSON.parse,
  } = options || {}

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