ronantakizawa/codereview-bench huggingface dataset:
Repository: storybookjs/storybook
File: code/core/src/docs-tools/argTypes/convert/typescript/convert.ts (TypeScript)

Code before:
};

export const convert = (type: TSType): SBType | void => {
  const { name, raw } = type;
  const base: any = {};

  if (typeof raw !== 'undefined') {
    base.raw = raw;
  }
  switch (type.name) {
    case 'string':
    case 'number':
    case 'symbol':
    case 'boolean': {
      return { ...base, name };
    }
    case 'Array': {
      return { ...base, name: 'array', value: type.elements.map(convert) };
    }
    case 'signature':
      return { ...base, ...convertSig(type) };
    case 'union':
      let result;
      // Filter out undefined from optional props when checking for enum
      const nonUndefinedElements = type.elements?.filter((element) => element.name !== 'undefined');
      if (nonUndefinedElements?.length > 0 && nonUndefinedElements.every((element) => element.name === 'literal')) {
        result = {
          ...base,
          name: 'enum',
          // @ts-expect-error fix types
          value: nonUndefinedElements.map((v) => parseLiteral(v.value)),
        };
      } else {
        result = { ...base, name, value: type.elements?.map(convert) };
      }
      return result;
    case 'intersection':
      return { ...base, name, value: type.elements?.map(convert) };
    default:
      return { ...base, name: 'other', value: name };
  }
};


Review comment (suggestion):
Type errors here if `nonUndefinedElements` is undefined.

Code after:
      return {
        name: 'object',
        value: values,
      };
    default:
      throw new UnknownArgTypesError({ type, language: 'Typescript' });
  }
};

export const convert = (type: TSType): SBType | void => {
  const { name, raw } = type;
  const base: any = {};

  if (typeof raw !== 'undefined') {
    base.raw = raw;
  }
  switch (type.name) {
    case 'string':
    case 'number':
    case 'symbol':
    case 'boolean': {
      return { ...base, name };
    }
    case 'Array': {
      return { ...base, name: 'array', value: type.elements.map(convert) };
    }
    case 'signature':
      return { ...base, ...convertSig(type) };
    case 'union': {
      const nonUndefinedElements = type.elements.filter((element) => !isUndefined(element));
      const allLiterals = nonUndefinedElements.length > 0 && nonUndefinedElements.every(isLiteral);

      if (allLiterals) {
        // TypeScript can't infer from .every(), so we filter again with the type guard
        const literalElements = nonUndefinedElements.filter(isLiteral);
        return {
          ...base,
          name: 'enum',
          value: literalElements.map((element) => parseLiteral(element.value)),
        };
      }
      return { ...base, name, value: type.elements.map(convert) };
    }
    case 'intersection':
      return { ...base, name, value: type.elements.map(convert) };
    default:
      return { ...base, name: 'other', value: name };
  }
};
Repository: storybookjs/storybook
File: code/core/src/docs-tools/argTypes/convert/typescript/convert.ts (TypeScript)

Code before:

  if (typeof raw !== 'undefined') {
    base.raw = raw;
  }
  switch (type.name) {
    case 'string':
    case 'number':
    case 'symbol':
    case 'boolean': {
      return { ...base, name };
    }
    case 'Array': {
      return { ...base, name: 'array', value: type.elements.map(convert) };
    }
    case 'signature':
      return { ...base, ...convertSig(type) };
    case 'union':
      let result;
      // Filter out undefined from optional props when checking for enum
      const nonUndefinedElements = type.elements?.filter((element) => element.name !== 'undefined');
      if (nonUndefinedElements?.length > 0 && nonUndefinedElements.every((element) => element.name === 'literal')) {
        result = {
          ...base,
          name: 'enum',
          // @ts-expect-error fix types
          value: nonUndefinedElements.map((v) => parseLiteral(v.value)),
        };
      } else {
        result = { ...base, name, value: type.elements?.map(convert) };
      }
      return result;
    case 'intersection':
      return { ...base, name, value: type.elements?.map(convert) };
    default:
      return { ...base, name: 'other', value: name };
  }
};


Review comment (bug):
This might be fixable if the code above used type guard functions. The fact that you found a type bug in a code path with `@ts-expect-error` hints that we really want to remove `@ts-expect-error` and actually get types right this time (without any casting, just with type guards).

Code after:
      throw new UnknownArgTypesError({ type, language: 'Typescript' });
  }
};

export const convert = (type: TSType): SBType | void => {
  const { name, raw } = type;
  const base: any = {};

  if (typeof raw !== 'undefined') {
    base.raw = raw;
  }
  switch (type.name) {
    case 'string':
    case 'number':
    case 'symbol':
    case 'boolean': {
      return { ...base, name };
    }
    case 'Array': {
      return { ...base, name: 'array', value: type.elements.map(convert) };
    }
    case 'signature':
      return { ...base, ...convertSig(type) };
    case 'union': {
      const nonUndefinedElements = type.elements.filter((element) => !isUndefined(element));
      const allLiterals = nonUndefinedElements.length > 0 && nonUndefinedElements.every(isLiteral);

      if (allLiterals) {
        // TypeScript can't infer from .every(), so we filter again with the type guard
        const literalElements = nonUndefinedElements.filter(isLiteral);
        return {
          ...base,
          name: 'enum',
          value: literalElements.map((element) => parseLiteral(element.value)),
        };
      }
      return { ...base, name, value: type.elements.map(convert) };
    }
    case 'intersection':
      return { ...base, name, value: type.elements.map(convert) };
    default:
      return { ...base, name: 'other', value: name };
  }
};
Repository: storybookjs/storybook
File: code/addons/docs/src/blocks/components/ArgsTable/ArgValue.tsx (TypeScript)

Code before:
  background: 'none',
  border: 'none',
}));

const Expandable = styled.div(codeCommon, ({ theme }) => ({
  fontFamily: theme.typography.fonts.mono,
  color: theme.color.secondary,
  fontSize: theme.typography.size.s1, // overrides codeCommon
  margin: 0,
  whiteSpace: 'nowrap',
  display: 'flex',
  alignItems: 'center',
}));

const Detail = styled.div(({ theme }) => ({
  minWidth: 200,
  maxWidth: 800,
  paddingRight: 16,
  // Don't remove the mono fontFamily here even if it seems useless, this is used by the browser to calculate the length of a "ch" unit.
  fontFamily: theme.typography.fonts.mono,
  fontSize: theme.typography.size.s1,
  // Most custom stylesheet will reset the box-sizing to "border-box" and will break the tooltip.
  boxSizing: 'content-box',

  '& code': {
    padding: '0 !important',
  },
}));

const ChevronUpIcon = styled(ChevronSmallUpIcon)({
  marginLeft: 4,
});

const ChevronDownIcon = styled(ChevronSmallDownIcon)({
  marginLeft: 4,
});

const EmptyArg = () => {
  return <span>-</span>;
};

const ArgText: FC<ArgTextProps> = ({ text, simple }) => {
  return <Text simple={simple}>{text}</Text>;
};

const getSummaryItems = (summary: string) => {
  if (!summary) {
    return [summary];
  }
  const splittedItems = summary.split('|');
  const summaryItems = splittedItems.map((value) => value.trim());

Review comment (suggestion):
* [ ] I'll need to change this into a `button` for WCAG compliance

Code after:
  margin: '0 0 4px',
  padding: '1px 6px',
  background: 'none',
  border: 'none',
}));

const Expandable = styled.button(codeCommon, ({ theme }) => ({
  fontFamily: theme.typography.fonts.mono,
  color: theme.color.secondary,
  fontSize: theme.typography.size.s1, // overrides codeCommon
  margin: 0,
  whiteSpace: 'nowrap',
  display: 'flex',
  alignItems: 'center',
  cursor: 'pointer',
  '&:hover': {
    border:
      theme.base === 'light' ? '1px solid hsl(0 0 0 / 0.15)' : '1px solid hsl(0 0 100 / 0.15)',
  },
  '&:focus-visible': {
    outline: `2px solid ${theme.color.secondary}`,
    outlineOffset: '2px',
  },
}));

const Detail = styled.div(({ theme }) => ({
  minWidth: 200,
  maxWidth: 800,
  paddingRight: 16,
  // Don't remove the mono fontFamily here even if it seems useless, this is used by the browser to calculate the length of a "ch" unit.
  fontFamily: theme.typography.fonts.mono,
  fontSize: theme.typography.size.s1,
  // Most custom stylesheet will reset the box-sizing to "border-box" and will break the tooltip.
  boxSizing: 'content-box',

  '& code': {
    padding: '0 !important',
  },
}));

const ChevronUpIcon = styled(ChevronSmallUpIcon)({
  marginLeft: 4,
});

const ChevronDownIcon = styled(ChevronSmallDownIcon)({
  marginLeft: 4,
});

const EmptyArg = () => {
  return <span>-</span>;
};
Repository: storybookjs/storybook
File: code/addons/docs/src/blocks/components/ArgsTable/ArgRow.stories.tsx (TypeScript)

Code before:
  
  // Determine the longer string
  const maxLength = Math.max(lengthA, lengthB);
  const minLength = Math.min(lengthA, lengthB);
  
  // Calculate similarity score
  let matchCount = 0;
  for (let i = 0; i < minLength; i++) {
    if (a[i] === b[i]) {
      matchCount++;
    }
  }
  
  // Compute weighted average
  const similarity = (matchCount / maxLength) * 100;
  
  // Generate result string
  const result = \`Similarity: \${similarity.toFixed(2)}%, Length diff: \${Math.abs(lengthA - lengthB)}\`;
  
  return result;
}`,
        },
      },
    },
  },
};

const enumeration =
  '"search" | "arrow-to-bottom" | "arrow-to-right" | "bell" | "check" | "check-circle"';
export const Enum = {
  args: {
    row: {
      key: 'enum',
      name: 'Some enum',
      type: {
        required: true,
      },
      table: {
        type: {
          summary: enumeration,
        },
      },
    },
  },
};

const longEnumeration =
  '"search" | "arrow-to-bottom" | "arrow-to-right" | "bell" | "check" | "check-circle" | "chevron-up" | "chevron-down" | "chevron-left" | "chevron-right" | "envelope" | "exchange" | "file" | "file-check" | "file-import" | "file-pdf" | "file-times" | "pencil" | "question" | "question-circle" | "sitemap" | "user" | "times" | "plus" | "exclamation-triangle" | "trash-alt" | "long-arrow-up" | "long-arrow-down" | "long-arrow-left" | "long-arrow-right" | "external-link-alt" | "sticky-note" | "chart-line" | "spinner-third"';
export const LongEnum = {
  args: {
    row: {

Review comment (suggestion):
This should probably have a play function which opens the tooltip, so that we can visually test this. The `Func` story should do the same.

Code after:
          detail: `(a, b) => {
  // Calculate various metrics between a and b
  const lengthA = a.length;
  const lengthB = b.length;
  
  // Determine the longer string
  const maxLength = Math.max(lengthA, lengthB);
  const minLength = Math.min(lengthA, lengthB);
  
  // Calculate similarity score
  let matchCount = 0;
  for (let i = 0; i < minLength; i++) {
    if (a[i] === b[i]) {
      matchCount++;
    }
  }
  
  // Compute weighted average
  const similarity = (matchCount / maxLength) * 100;
  
  // Generate result string
  const result = \`Similarity: \${similarity.toFixed(2)}%, Length diff: \${Math.abs(lengthA - lengthB)}\`;
  
  return result;
}`,
        },
      },
    },
  },
  play: async ({ canvas }) => {
    const funcButton = canvas.getByRole('button', { name: 'func' });
    funcButton.click();
    expect(
      await screen.findByText(/Calculate various metrics between a and b/)
    ).toBeInTheDocument();
  },
} satisfies StoryObj<typeof ArgRow>;

const enumeration =
  '"search" | "arrow-to-bottom" | "arrow-to-right" | "bell" | "check" | "check-circle"';
export const Enum = {
  args: {
    row: {
      key: 'enum',
      name: 'Some enum',
      type: {
        required: true,
      },
      table: {
        type: {
          summary: enumeration,
Repository: storybookjs/storybook
File: code/core/src/toolbar/components/ToolbarMenuSelect.tsx (TypeScript)

Code before:
          };
        } else {
          return {
            title: itemTitle,
            value: item.value,
            icon: iconComponent,
          };
        }
      });

    // FIXME: for SB 10 we would want description to become an aria-description, and to add an
    // ariaLabel prop to tools with an automigration switching current description to ariaLabel
    const ariaLabel = description || title || name || id;

    return (
      <Select
        defaultOptions={[currentValue]}
        options={options}
        disabled={isOverridden}
        ariaLabel={ariaLabel}
        tooltip={ariaLabel}
        resetLabel={resetLabel}
        onReset={() => updateGlobals({ [id]: '_reset' })}
        onSelect={(selected) => updateGlobals({ [id]: selected })}
        icon={icon && <Icons icon={icon} __suppressDeprecationWarning={true} />}
        showSelectedLabel={false}
      >
        {title}
      </Select>
    );
  }
);


Review comment (suggestion):
```suggestion
        showSelectedLabel={dynamicTitle}
```

otherwise it'll _never_ update

Code after:
                <ToolbarMenuItemContainer>
                  {iconComponent}
                  <ToolbarMenuItemMiddle>{item.title ?? item.value}</ToolbarMenuItemMiddle>
                  {item.right}
                </ToolbarMenuItemContainer>
              ),
            };
          } else {
            return {
              title: itemTitle,
              value: item.value,
              icon: iconComponent,
            };
          }
        }),
    [items]
  );

  React.useEffect(() => {
    if (shortcuts) {
      const length = options.length;
      void registerShortcuts(api, id, {
        next: {
          ...shortcuts.next,
          action: () => {
            const idx = options.findIndex((i) => i.value === globals[id]);
            const nextIdx = idx < 0 ? 0 : (idx + 1) % length;
            updateGlobals({ [id]: options[nextIdx].value });
          },
        },
        previous: {
          ...shortcuts.previous,
          action: () => {
            const idx = options.findIndex((i) => i.value === globals[id]);
            const previousIdx = idx < 0 ? length - 1 : (idx + length - 1) % length;
            updateGlobals({ [id]: options[previousIdx].value });
          },
        },
        reset: {
          ...shortcuts.reset,
          action: () => {
            updateGlobals({ [id]: undefined });
          },
        },
      });
    }
  }, [api, id, shortcuts, globals, options, updateGlobals]);

  // FIXME: for SB 10 we would want description to become an aria-description, and to add an
  // ariaLabel prop to tools with an automigration switching current description to ariaLabel
  const ariaLabel = description || title || name || id;
Repository: storybookjs/storybook
File: code/core/src/core-server/utils/open-browser/opener.ts (TypeScript)

Code before:

const Actions = Object.freeze({
  NONE: 0,
  BROWSER: 1,
  SCRIPT: 2,
});

function getBrowserEnv() {
  // Attempt to honor this environment variable.
  // It is specific to the operating system.
  // See https://github.com/sindresorhus/open#app for documentation.
  const value = process.env.BROWSER;
  const args = process.env.BROWSER_ARGS ? process.env.BROWSER_ARGS.split(' ') : [];
  let action;
  if (!value) {
    // Default.
    action = Actions.BROWSER;
  } else if (value.toLowerCase() === 'none') {
    action = Actions.NONE;
  } else if (
    value.toLowerCase().endsWith('.js') ||
    value.toLowerCase().endsWith('.mjs') ||
    value.toLowerCase().endsWith('.cjs') ||
    value.toLowerCase().endsWith('.sh')
  ) {
    action = Actions.SCRIPT;
  } else {
    action = Actions.BROWSER;
  }
  return { action, value, args };
}

function executeNodeScript(scriptPath: string, url: string) {
  const extraArgs = process.argv.slice(2);
  const child = spawn(process.execPath, [scriptPath, ...extraArgs, url], {
    stdio: 'inherit',
  });
  child.on('close', (code) => {
    if (code !== 0) {
      console.log();
      console.log(picocolors.red('The script specified as BROWSER environment variable failed.'));
      console.log(`${picocolors.cyan(scriptPath)} exited with code ${code}.`);
      console.log();
      return;
    }
  });
  return true;
}

function startBrowserProcess(
  browser: App | readonly App[] | undefined,

Review comment (bug):
Attn: this will cause a shell script to be passed on to a Node runtime. It will most likely cause bugs for other users.

Code after:

// https://github.com/sindresorhus/open#app
const OSX_CHROME = 'google chrome';

enum Actions {
  NONE = 0,
  BROWSER = 1,
  SCRIPT = 2,
  SHELL_SCRIPT = 3,
}

function getBrowserEnv():
  | {
      action: Actions.SCRIPT | Actions.SHELL_SCRIPT;
      value: string;
      args: string[];
    }
  | {
      action: Actions.BROWSER;
      value?: string;
      args: string[];
    }
  | {
      action: Actions.NONE;
      value?: undefined;
      args?: undefined;
    } {
  // Attempt to honor this environment variable.
  // It is specific to the operating system.
  // See https://github.com/sindresorhus/open#app for documentation.
  const value = process.env.BROWSER;
  const args = process.env.BROWSER_ARGS ? process.env.BROWSER_ARGS.split(' ') : [];

  // Default.
  if (!value) {
    return { action: Actions.BROWSER, args };
  }

  if (value.toLowerCase() === 'none') {
    return { action: Actions.NONE };
  }

  if (
    value.toLowerCase().endsWith('.js') ||
    value.toLowerCase().endsWith('.mjs') ||
    value.toLowerCase().endsWith('.cjs') ||
    value.toLowerCase().endsWith('.ts')
  ) {
    return { action: Actions.SCRIPT, value, args };
  }
Repository: storybookjs/storybook
File: code/core/src/core-server/utils/open-browser/opener.ts (TypeScript)

Code before:
// https://github.com/sindresorhus/open#app
const OSX_CHROME = 'google chrome';

const Actions = Object.freeze({
  NONE: 0,
  BROWSER: 1,
  SCRIPT: 2,
  SHELL_SCRIPT: 3,
});

function getBrowserEnv() {
  // Attempt to honor this environment variable.
  // It is specific to the operating system.
  // See https://github.com/sindresorhus/open#app for documentation.
  const value = process.env.BROWSER;
  const args = process.env.BROWSER_ARGS ? process.env.BROWSER_ARGS.split(' ') : [];
  let action;
  if (!value) {
    // Default.
    action = Actions.BROWSER;
  } else if (value.toLowerCase() === 'none') {
    action = Actions.NONE;
  } else if (
    value.toLowerCase().endsWith('.js') ||
    value.toLowerCase().endsWith('.mjs') ||
    value.toLowerCase().endsWith('.cjs')
  ) {
    action = Actions.SCRIPT;
  } else if (value.toLowerCase().endsWith('.sh')) {
    action = Actions.SHELL_SCRIPT;
  } else {
    action = Actions.BROWSER;
  }
  return { action, value, args };
}

class BrowserEnvError extends StorybookError {
  constructor(message: string) {
    super({
      category: 'CORE_SERVER',
      code: 1,
      message,
      name: 'BrowserEnvError',
    });
  }
}

function attachCloseHandler(child: ReturnType<typeof spawn>, scriptPath: string) {
  child.on('close', (code) => {
    if (code !== 0) {
      logger.error(

Review comment (suggestion):
This is good and should be kept. We could even extend to `.ts` for Node 24+.

Code after:
const OSX_CHROME = 'google chrome';

enum Actions {
  NONE = 0,
  BROWSER = 1,
  SCRIPT = 2,
  SHELL_SCRIPT = 3,
}

function getBrowserEnv():
  | {
      action: Actions.SCRIPT | Actions.SHELL_SCRIPT;
      value: string;
      args: string[];
    }
  | {
      action: Actions.BROWSER;
      value?: string;
      args: string[];
    }
  | {
      action: Actions.NONE;
      value?: undefined;
      args?: undefined;
    } {
  // Attempt to honor this environment variable.
  // It is specific to the operating system.
  // See https://github.com/sindresorhus/open#app for documentation.
  const value = process.env.BROWSER;
  const args = process.env.BROWSER_ARGS ? process.env.BROWSER_ARGS.split(' ') : [];

  // Default.
  if (!value) {
    return { action: Actions.BROWSER, args };
  }

  if (value.toLowerCase() === 'none') {
    return { action: Actions.NONE };
  }

  if (
    value.toLowerCase().endsWith('.js') ||
    value.toLowerCase().endsWith('.mjs') ||
    value.toLowerCase().endsWith('.cjs') ||
    value.toLowerCase().endsWith('.ts')
  ) {
    return { action: Actions.SCRIPT, value, args };
  }

  if (value.toLowerCase().endsWith('.sh')) {
    return { action: Actions.SHELL_SCRIPT, value, args };
Repository: storybookjs/storybook
File: code/core/src/preview-api/modules/store/csf/getValuesFromGlobalTypes.ts (TypeScript)

Code before:
import type { GlobalTypes, Globals } from 'storybook/internal/types';

export const getValuesFromGlobalTypes = (argTypes: GlobalTypes = {}): Globals =>
  Object.entries(argTypes).reduce<Globals>((acc, [arg, { defaultValue }]) => {
    if (typeof defaultValue !== 'undefined') {
      acc[arg] = defaultValue;
    }
    return acc;
  }, {});


Review comment (suggestion):
```suggestion
export const getValuesFromGlobalTypes = (globalTypes: GlobalTypes = {}): Globals =>
  Object.entries(globalTypes).reduce<Globals>((acc, [arg, { defaultValue }]) => {
    if (typeof defaultValue !== 'undefined') {
      acc[arg] = defaultValue;
    }
    return acc;
  }, {});
```

Code after:
import type { GlobalTypes, Globals } from 'storybook/internal/types';

export const getValuesFromGlobalTypes = (globalTypes: GlobalTypes = {}): Globals =>
  Object.entries(globalTypes).reduce<Globals>((acc, [arg, { defaultValue }]) => {
    if (typeof defaultValue !== 'undefined') {
      acc[arg] = defaultValue;
    }
    return acc;
  }, {});
Repository: storybookjs/storybook
File: code/core/src/toolbar/types.ts (TypeScript)

Code before:
import type { InputType } from 'storybook/internal/types';

import type { IconsProps } from '../components/components/icon/icon';

export type ToolbarShortcutType = 'next' | 'previous' | 'reset';

export type ToolbarItemType = 'item' | 'reset';

export interface ToolbarShortcutConfig {
  label: string;
  keys: string[];
}

export type ToolbarShortcuts = Record<ToolbarShortcutType, ToolbarShortcutConfig>;

export interface ToolbarItem {
  value?: string;
  icon?: IconsProps['icon'];
  right?: string;
  title?: string;
  hideIcon?: boolean;
  type?: ToolbarItemType;
}

export interface NormalizedToolbarConfig {
  /** The label to show for this toolbar item */
  title?: string;
  /** Choose an icon to show for this toolbar item */
  icon?: IconsProps['icon'];
  /** Set to true to prevent default update of icon to match any present selected items icon */
  preventDynamicIcon?: boolean;
  items: ToolbarItem[];
  shortcuts?: ToolbarShortcuts;
  /** Change title based on selected value */
  dynamicTitle?: boolean;
}

export type NormalizedToolbarArgType = {
  name: string;
  description: string;
  defaultValue?: any;
  toolbar: NormalizedToolbarConfig;
};

export type ToolbarConfig = Omit<NormalizedToolbarConfig, 'items'> & {
  items: (string | ToolbarItem)[];
};

export type ToolbarArgType = {
  name?: string;
  description?: string;
  defaultValue?: any;
  toolbar?: ToolbarConfig;
};

export type ToolbarMenuProps = NormalizedToolbarArgType & { id: string };


Review comment (suggestion):
```suggestion
  [name: string]: any;
};
```

for looseness? add a deprecation comment...

Code after:
  /** Change title based on selected value */
  dynamicTitle?: boolean;
}

export type NormalizedToolbarArgType = {
  name: string;
  description: string;
  defaultValue?: any;
  toolbar: NormalizedToolbarConfig;
};

export type ToolbarConfig = Omit<NormalizedToolbarConfig, 'items'> & {
  items: (string | ToolbarItem)[];
};

export type ToolbarArgType = {
  name?: string;
  description?: string;
  defaultValue?: any;
  toolbar?: ToolbarConfig;
  /**
   * @deprecated This loose index signature has been added for compatibility with InputType, and
   *   will be removed in Storybook 11
   */
  [key: string]: any;
};

export type ToolbarMenuProps = NormalizedToolbarArgType & { id: string };