ronantakizawa/codereview-bench huggingface dataset:
Repository: nextui-org/nextui
File: packages/components/slider/src/use-slider.ts (TypeScript)

Code before:
  } = props;

  const isFixedValue = minValue === maxValue;

  if (isFixedValue) {
    warn("Min and max values should not be the same. This may cause unexpected behavior.");
  }

  const Component = as || "div";
  const shouldFilterDOMProps = typeof Component === "string";
  const disableAnimation =
    originalProps?.disableAnimation ?? globalContext?.disableAnimation ?? false;

  const domRef = useDOMRef(ref);
  const trackRef = useRef<HTMLDivElement>(null);

  const numberFormatter = useNumberFormatter(formatOptions);
  const {direction} = useLocale();

  const clampValue = useCallback(
    (valueToClamp: number) => {
      if (isFixedValue) return minValue;

      return Math.min(Math.max(valueToClamp, minValue), maxValue);
    },
    [minValue, maxValue, isFixedValue],
  );

  const validatedValue = useMemo(() => {
    if (isFixedValue) return minValue;
    if (valueProp === undefined) return undefined;

    if (Array.isArray(valueProp)) {
      return valueProp.map(clampValue);
    }

    return clampValue(valueProp);
  }, [valueProp, clampValue, isFixedValue, minValue]);

  const state = useSliderState({
    ...otherProps,
    value: validatedValue,
    defaultValue: isFixedValue ? minValue : otherProps.defaultValue,
    isDisabled: originalProps?.isDisabled ?? false,
    orientation,
    step,
    minValue,
    maxValue,
    numberFormatter,
    onChange: isFixedValue ? () => {} : onChange,
    onChangeEnd: isFixedValue ? () => {} : onChangeEnd,

Review comment (suggestion):
Undo it. `clampValue` is called from `validatedValue`, where you return `minValue` if `isFixedValue` is `true`. Hence, the change you added won't take any effect. 

Code after:
  if (isFixedValue) {
    warn("Min and max values should not be the same. This may cause unexpected behavior.");
  }

  const Component = as || "div";
  const shouldFilterDOMProps = typeof Component === "string";
  const disableAnimation =
    originalProps?.disableAnimation ?? globalContext?.disableAnimation ?? false;

  const domRef = useDOMRef(ref);
  const trackRef = useRef<HTMLDivElement>(null);

  const numberFormatter = useNumberFormatter(formatOptions);
  const {direction} = useLocale();

  const clampValue = useCallback(
    (valueToClamp: number) => {
      return Math.min(Math.max(valueToClamp, minValue), maxValue);
    },
    [minValue, maxValue],
  );

  const validatedValue = useMemo(() => {
    if (isFixedValue) return minValue;

    if (valueProp === undefined) return undefined;

    if (Array.isArray(valueProp)) {
      return valueProp.map(clampValue);
    }

    return clampValue(valueProp);
  }, [valueProp, clampValue, isFixedValue, minValue]);

  const state = useSliderState({
    ...otherProps,
    value: validatedValue,
    isDisabled: originalProps?.isDisabled ?? false,
    orientation,
    step,
    minValue,
    maxValue,
    numberFormatter,
    onChange,
    onChangeEnd,
  });

  const tooltipProps: Partial<TooltipProps> = {
    offset: 5,
    delay: 0,
    size: "sm",
Repository: nextui-org/nextui
File: packages/core/theme/src/components/table.ts (TypeScript)

Code before:
      },
      fixed: {
        table: "table-fixed",
      },
    },
    radius: {
      none: {
        wrapper: "rounded-none",
        th: [
          "!rounded-none",
          "first:!rounded-none",
          "last:!rounded-none",
          "before:!rounded-none",
          "group-data-[odd=true]/tr:before:!rounded-none",
          "group-data-[odd=true]/tr:first:before:!rounded-none",
          "group-data-[odd=true]/tr:last:before:!rounded-none",
        ],
        td: [
          "!rounded-none",
          "first:!rounded-none",
          "last:!rounded-none",
          "before:!rounded-none",
          "group-data-[odd=true]/tr:before:!rounded-none",
          "group-data-[odd=true]/tr:first:before:!rounded-none",
          "group-data-[odd=true]/tr:last:before:!rounded-none",
        ],
      },
      sm: {
        wrapper: "rounded-small",
      },
      md: {
        wrapper: "rounded-medium",
      },
      lg: {
        wrapper: "rounded-large",
      },
    },
    shadow: {
      none: {
        wrapper: "shadow-none",
      },
      sm: {
        wrapper: "shadow-small",
      },
      md: {
        wrapper: "shadow-medium",
      },
      lg: {
        wrapper: "shadow-large",
      },
    },

Review comment (suggestion):
We can just move the entire block of `radius` right after `isMultiSelectable` to avoid using `!`. Also please change to following

```ts
radius: {
  none: {
    wrapper: "rounded-none",
    th: [
      "first:rounded-s-none",
      "first:before:rounded-s-none",
      "last:rounded-e-none",
      "last:before:rounded-e-none",
    ],
    td: [
      "first:before:rounded-s-none",
      "last:before:rounded-e-none",
      "group-data-[first=true]/tr:first:before:rounded-ss-none",
      "group-data-[first=true]/tr:last:before:rounded-se-none",
      "group-data-[last=true]/tr:first:before:rounded-es-none",
      "group-data-[last=true]/tr:last:before:rounded-ee-none",
    ],
  },
```

Code after:
      },
      fixed: {
        table: "table-fixed",
      },
    },
    shadow: {
      none: {
        wrapper: "shadow-none",
      },
      sm: {
        wrapper: "shadow-small",
      },
      md: {
        wrapper: "shadow-medium",
      },
      lg: {
        wrapper: "shadow-large",
      },
    },
    hideHeader: {
      true: {
        thead: "hidden",
      },
    },
    isStriped: {
      true: {
        td: [
          "group-data-[odd=true]/tr:before:bg-default-100",
          "group-data-[odd=true]/tr:before:opacity-100",
          "group-data-[odd=true]/tr:before:-z-10",
        ],
      },
    },
    isCompact: {
      true: {
        td: "py-1",
      },
      false: {},
    },
    isHeaderSticky: {
      true: {
        thead: "sticky top-0 z-20 [&>tr]:first:shadow-small",
      },
    },
    isSelectable: {
      true: {
        tr: "cursor-default",
        td: [
          "group-aria-[selected=false]/tr:group-data-[hover=true]/tr:before:bg-default-100",
          "group-aria-[selected=false]/tr:group-data-[hover=true]/tr:before:opacity-70",
        ],
Repository: nextui-org/nextui
File: packages/core/system-rsc/src/extend-variants.d.ts (TypeScript)

Code before:
    : C extends new (...args: any[]) => infer I
      ? I
      : any;

export type ExtendVariants = <
  C extends keyof JSX.IntrinsicElements | JSXElementConstructor<any>,
  CP extends ComponentProps<C> = ComponentProps<C>,
  S extends ComponentSlots<CP> = ComponentSlots<CP>,
  V extends ComposeVariants<CP, S> = ComposeVariants<CP, S>,
  SV extends SuggestedVariants<CP, S> = SuggestedVariants<CP, S>,
  DV extends DefaultVariants<V, SV, S> = DefaultVariants<V, SV, S>,
  CV extends CompoundVariants<V, SV, ComponentSlots<CP>> = CompoundVariants<
    V,
    SV,
    ComponentSlots<CP>
  >,
>(
  BaseComponent: C,
  styles: {
    variants?: V;
    defaultVariants?: DV;
    compoundVariants?: CV;
    slots?: S;
  },
  opts?: Options,
) => _heroui_system.InternalForwardRefRenderFunction<
  C,
  {
    [K in Exclude<keyof (CP & V), "ref" | "as">]?:
      | (K extends keyof CP ? CP[K] : never)
      | (K extends keyof V ? StringToBoolean<keyof NonNullable<V[K]>> : never);
  }
>;

// main function
export declare const extendVariants: ExtendVariants;


Review comment (suggestion):
I think we may just do `import type {InternalForwardRefRenderFunction} from "./types.js";` .

Code after:
    : C extends new (...args: any[]) => infer I
      ? I
      : any;

export type ExtendVariants = <
  C extends keyof JSX.IntrinsicElements | JSXElementConstructor<any>,
  CP extends ComponentProps<C> = ComponentProps<C>,
  S extends ComponentSlots<CP> = ComponentSlots<CP>,
  V extends ComposeVariants<CP, S> = ComposeVariants<CP, S>,
  SV extends SuggestedVariants<CP, S> = SuggestedVariants<CP, S>,
  DV extends DefaultVariants<V, SV, S> = DefaultVariants<V, SV, S>,
  CV extends CompoundVariants<V, SV, ComponentSlots<CP>> = CompoundVariants<
    V,
    SV,
    ComponentSlots<CP>
  >,
>(
  BaseComponent: C,
  styles: {
    variants?: V;
    defaultVariants?: DV;
    compoundVariants?: CV;
    slots?: S;
  },
  opts?: Options,
) => InternalForwardRefRenderFunction<
  C,
  {
    [K in Exclude<keyof (CP & V), "ref" | "as">]?:
      | (K extends keyof CP ? CP[K] : never)
      | (K extends keyof V ? StringToBoolean<keyof NonNullable<V[K]>> : never);
  }
>;

// main function
export declare const extendVariants: ExtendVariants;
Repository: nextui-org/nextui
File: packages/components/link/src/use-link.ts (TypeScript)

Code before:
  /**
   * Whether to show the icon when the link is external.
   * @default false
   */
  showAnchorIcon?: boolean;
  /**
   * The icon to display right after the link.
   * @default <LinkIcon />
   */
  anchorIcon?: React.ReactNode;
  /**
   * The native link click event handler.
   * use `onPress` instead.
   * @deprecated
   */
  onClick?: MouseEventHandler<HTMLAnchorElement>;
}

export type UseLinkProps = Props & AriaLinkProps;

export function useLink(originalProps: UseLinkProps) {
  const globalContext = useProviderContext();
  const resolvedProps =
    globalContext?.useHref && originalProps.href
      ? {...originalProps, href: globalContext.useHref(originalProps.href)}
      : originalProps;

  const [props, variantProps] = mapPropsVariants(resolvedProps, link.variantKeys);

  const {
    ref,
    as,
    children,
    anchorIcon,
    isExternal = false,
    showAnchorIcon = false,
    autoFocus = false,
    className,
    onPress,
    onPressStart,
    onPressEnd,
    onClick,
    ...otherProps
  } = props;

  const Component = as || "a";

  const domRef = useDOMRef(ref);
  const disableAnimation =
    originalProps?.disableAnimation ?? globalContext?.disableAnimation ?? false;


Review comment (suggestion):
This logic seems to be covered in `useLinkProps` in `packages/hooks/use-aria-link/src/index.ts` already. Could you please cross check the react-spectrum implementation (`packages/@react-aria/link/src/useLink.ts`)? From what I see, they look same.

Code after:
  /**
   * Whether to show the icon when the link is external.
   * @default false
   */
  showAnchorIcon?: boolean;
  /**
   * The icon to display right after the link.
   * @default <LinkIcon />
   */
  anchorIcon?: React.ReactNode;
  /**
   * The native link click event handler.
   * use `onPress` instead.
   * @deprecated
   */
  onClick?: MouseEventHandler<HTMLAnchorElement>;
}

export type UseLinkProps = Props & AriaLinkProps;

export function useLink(originalProps: UseLinkProps) {
  const globalContext = useProviderContext();

  const [props, variantProps] = mapPropsVariants(originalProps, link.variantKeys);

  const {
    ref,
    as,
    children,
    anchorIcon,
    isExternal = false,
    showAnchorIcon = false,
    autoFocus = false,
    className,
    onPress,
    onPressStart,
    onPressEnd,
    onClick,
    ...otherProps
  } = props;

  const Component = as || "a";

  const domRef = useDOMRef(ref);
  const disableAnimation =
    originalProps?.disableAnimation ?? globalContext?.disableAnimation ?? false;

  // use `@heroui/use-aria-link` to suppress onClick deprecation warning
  const {linkProps} = useAriaLink(
    {
      ...otherProps,
Repository: nicbarker/clay
File: clay.h (C/C++)

Code before:
    uint32_t base = seed;

    for (int i = 0; i < key.length; i++) {
        base += key.chars[i];
        base += (base << 10);
        base ^= (base >> 6);
    }
    hash = base;
    hash += offset;
    hash += (hash << 10);
    hash ^= (hash >> 6);

    hash += (hash << 3);
    base += (base << 3);
    hash ^= (hash >> 11);
    base ^= (base >> 11);
    hash += (hash << 15);
    base += (base << 15);
    #ifdef CLAY_DEBUG
    LAST_HASH = key;
    LAST_HASH.length = (int)offset;
    #endif
    return (Clay_ElementId) { .stringId = key, .id = hash + 1, .offset = offset, .baseId = base + 1 }; // Reserve the hash result of zero as "null id"
}

uint32_t Clay__GetParentId(void) {
    return Clay__openLayoutElement ? Clay__openLayoutElement->id : 0;
}

Clay_ElementId Clay__Rehash(Clay_ElementId elementId, uint32_t number) {
    uint32_t id = elementId.baseId;
    id += number;
    id += (id << 10);
    id ^= (id >> 6);

    id += (id << 3);
    id ^= (id >> 11);
    id += (id << 15);
    return (Clay_ElementId) { .stringId = elementId.stringId, .id = id, .offset = number, .baseId = elementId.baseId };
}

uint32_t Clay__RehashWithNumber(uint32_t id, uint32_t number) {
    id += number;
    id += (id << 10);
    id ^= (id >> 6);

    id += (id << 3);
    id ^= (id >> 11);
    id += (id << 15);
    return id;
}

Review comment (suggestion):
```suggestion
uint32_t Clay__GetOpenLayoutElementId(void) {
```

Code after:
    for (int i = 0; i < key.length; i++) {
        base += key.chars[i];
        base += (base << 10);
        base ^= (base >> 6);
    }
    hash = base;
    hash += offset;
    hash += (hash << 10);
    hash ^= (hash >> 6);

    hash += (hash << 3);
    base += (base << 3);
    hash ^= (hash >> 11);
    base ^= (base >> 11);
    hash += (hash << 15);
    base += (base << 15);
    #ifdef CLAY_DEBUG
    LAST_HASH = key;
    LAST_HASH.length = (int)offset;
    #endif
    return (Clay_ElementId) { .stringId = key, .id = hash + 1, .offset = offset, .baseId = base + 1 }; // Reserve the hash result of zero as "null id"
}

uint32_t Clay__GetOpenLayoutElementId(void) {
    return Clay__openLayoutElement ? Clay__openLayoutElement->id : 0;
}

Clay_ElementId Clay__Rehash(Clay_ElementId elementId, uint32_t number) {
    uint32_t id = elementId.baseId;
    id += number;
    id += (id << 10);
    id ^= (id >> 6);

    id += (id << 3);
    id ^= (id >> 11);
    id += (id << 15);
    return (Clay_ElementId) { .stringId = elementId.stringId, .id = id, .offset = number, .baseId = elementId.baseId };
}

uint32_t Clay__RehashWithNumber(uint32_t id, uint32_t number) {
    id += number;
    id += (id << 10);
    id ^= (id >> 6);

    id += (id << 3);
    id ^= (id >> 11);
    id += (id << 15);
    return id;
}

uint32_t Clay__HashTextWithConfig(Clay_String *text, Clay_TextElementConfig *config) {
Repository: nicbarker/clay
File: renderers/SDL3/clay_renderer_SDL3.c (C)

Code before:
                    SDL_RenderArc(renderer, (SDL_FPoint){centerX, centerY}, clampedRadii.bottomRight,
                        0.0f, 90.0f, config->width.bottom, config->color);
                }

            } break;
            case CLAY_RENDER_COMMAND_TYPE_SCISSOR_START: {
                Clay_BoundingBox boundingBox = rcmd->boundingBox;
                currentClippingRectangle = (SDL_Rect) {
                        .x = boundingBox.x,
                        .y = boundingBox.y,
                        .w = boundingBox.width,
                        .h = boundingBox.height,
                };
                SDL_SetRenderClipRect(renderer, &currentClippingRectangle);
                break;
            }
            case CLAY_RENDER_COMMAND_TYPE_SCISSOR_END: {
                SDL_SetRenderClipRect(renderer, NULL);
                break;
            }
            case CLAY_RENDER_COMMAND_TYPE_IMAGE: {
                Clay_ImageRenderData *config = &rcmd->renderData.image;
                const SDL_Texture *texture = SDL_CreateTextureFromSurface(renderer, config->imageData);
                const SDL_FRect dest = { rect.x, rect.y, rect.w, rect.h };

                SDL_RenderTexture(renderer, texture, NULL, &dest);
                break;
            }
            default:
                SDL_Log("Unknown render command type: %d", rcmd->commandType);
        }
    }
}


Review comment (suggestion):
After it's rendered, the texture should be destroyed here:

```suggestion
                SDL_RenderTexture(renderer, texture, NULL, &dest);
                SDL_DestroyTexture(texture);
```

Code after:
                        0.0f, 90.0f, config->width.bottom, config->color);
                }

            } break;
            case CLAY_RENDER_COMMAND_TYPE_SCISSOR_START: {
                Clay_BoundingBox boundingBox = rcmd->boundingBox;
                currentClippingRectangle = (SDL_Rect) {
                        .x = boundingBox.x,
                        .y = boundingBox.y,
                        .w = boundingBox.width,
                        .h = boundingBox.height,
                };
                SDL_SetRenderClipRect(rendererData->renderer, &currentClippingRectangle);
                break;
            }
            case CLAY_RENDER_COMMAND_TYPE_SCISSOR_END: {
                SDL_SetRenderClipRect(rendererData->renderer, NULL);
                break;
            }
            case CLAY_RENDER_COMMAND_TYPE_IMAGE: {
                SDL_Surface *image = (SDL_Surface *)rcmd->renderData.image.imageData;
                SDL_Texture *texture = SDL_CreateTextureFromSurface(rendererData->renderer, image);
                const SDL_FRect dest = { rect.x, rect.y, rect.w, rect.h };

                SDL_RenderTexture(rendererData->renderer, texture, NULL, &dest);
                SDL_DestroyTexture(texture);
                break;
            }
            default:
                SDL_Log("Unknown render command type: %d", rcmd->commandType);
        }
    }
}
Repository: nocobase/nocobase
File: packages/plugins/@nocobase/plugin-localization/src/client/index.tsx (TypeScript)

Code before:
/**
 * This file is part of the NocoBase (R) project.
 * Copyright (c) 2020-2024 NocoBase Co., Ltd.
 * Authors: NocoBase Team.
 *
 * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License.
 * For more information, please refer to: https://www.nocobase.com/agreement.
 */

import { Plugin } from '@nocobase/client';
// import { Localization } from './Localization';
import { lazy } from '@nocobase/client';
const { Localization } = lazy(() => import('./Localization'), 'Localization');
import { NAMESPACE } from './locale';
import { MissingKeyProvider } from './MissingKeyProvider';

export class PluginLocalizationClient extends Plugin {
  async load() {
    this.app.use(MissingKeyProvider);

    this.app.pluginSettingsManager.add(NAMESPACE, {
      title: `{{t("Localization", { ns: "${NAMESPACE}" })}}`,
      icon: 'GlobalOutlined',
      Component: Localization,
      aclSnippet: 'pm.localization.localization',
    });
  }
}

export default PluginLocalizationClient;


Review comment (suggestion):
这里用 Provider 的写法不太合适，可以这样

```ts
import { FlowEngineContext } from '@nocobase/flow-engine';

export class MissingKeyHandler {
  constructor(public context: FlowEngineContext) {}

  register() {}
}
```

其中

- `context.api` 就是 APIClient
- `context.flowSettingsEnabled` 是 designable
- `context.i18n` 是 i18n 实例，用这个

注册 MissingKeyHandler 可以这样

```ts
export class PluginLocalizationClient extends Plugin {
  async load() {
    const missingKeyHandler = new MissingKeyHandler(this.engine.context);
    missingKeyHandler.register();
  }
}
```

Code after:
/**
 * This file is part of the NocoBase (R) project.
 * Copyright (c) 2020-2024 NocoBase Co., Ltd.
 * Authors: NocoBase Team.
 *
 * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License.
 * For more information, please refer to: https://www.nocobase.com/agreement.
 */

import { Plugin } from '@nocobase/client';
// import { Localization } from './Localization';
import { lazy } from '@nocobase/client';
const { Localization } = lazy(() => import('./Localization'), 'Localization');
import { NAMESPACE } from './locale';
import { MissingKeyHandler } from './i18n-missing-handler';

export class PluginLocalizationClient extends Plugin {
  async load() {
    const missingKeyHandler = new MissingKeyHandler(this.engine.context);
    missingKeyHandler.register();

    this.app.pluginSettingsManager.add(NAMESPACE, {
      title: `{{t("Localization", { ns: "${NAMESPACE}" })}}`,
      icon: 'GlobalOutlined',
      Component: Localization,
      aclSnippet: 'pm.localization.localization',
    });
  }
}

export default PluginLocalizationClient;
Repository: nocobase/nocobase
File: packages/plugins/@nocobase/plugin-localization/src/server/actions/localizationTexts.ts (TypeScript)

Code before:
        value: plugin.alias || plugin.name,
        label: plugin.displayName,
      })),
    ],
  };
  await next();
};

const missing = async (ctx: Context, next: Next) => {
  const { keys }: any = ctx.request.body || {};

  const currentRoles = ctx.state.currentRoles || [];
  if (!currentRoles.includes('root')) {
    const role = await ctx.db.getRepository('roles').findOne({
      filter: {
        name: currentRoles,
        allowConfigure: true,
      },
    });
    if (!role) {
      ctx.throw(403, 'No permission');
    }
  }

  if (keys?.length > 0) {
    const plugin = ctx.app.pm?.get('localization');
    await plugin?.addNewTexts(keys.map((key) => ({ text: key.text, module: `resources.${key.ns}` })));
  }

  await next();
};

export default { list, missing };


Review comment (suggestion):
不是用 `allowConfigure: true` 来判断的，是用 `ui.*` 判断的。

<img width="1044" height="568" alt="Image" src="https://github.com/user-attachments/assets/9944c6c1-7f17-48f4-86a5-9014141ac2d4" />

<img width="1118" height="694" alt="Image" src="https://github.com/user-attachments/assets/1167758b-4ba2-450d-97e1-e2fcf934c180" />

Code after:
        value: plugin.alias || plugin.name,
        label: plugin.displayName,
      })),
    ],
  };
  await next();
};

const missing = async (ctx: Context, next: Next) => {
  const { keys, locale }: any = ctx.request.body || {};

  const currentRoles = ctx.state.currentRoles || [];
  if (!currentRoles.includes('root')) {
    const roles = await ctx.db.getRepository('roles').find({
      filter: {
        name: currentRoles,
      },
    });
    const hasUiSnippet = roles.some((role) => {
      const snippets = (role.get('snippets') as string[]) || [];
      return snippets.includes('ui.*');
    });
    if (!hasUiSnippet) {
      ctx.throw(403, 'No permission');
    }
  }

  if (keys?.length > 0) {
    const plugin = ctx.app.pm?.get('localization');
    const currentLocale = locale || ctx.get('X-Locale') || 'en-US';
    await plugin?.addNewTexts(
      keys.map((key) => ({ text: key.text, module: `resources.${key.ns}` })),
      {
        locale: currentLocale,
      },
    );
  }

  await next();
};

export default { list, missing };