ronantakizawa/codereview-bench huggingface dataset:
Repository: storybookjs/storybook
File: code/core/src/types/modules/core-common.ts (TypeScript)

Code before:
  id: string;
  url: string;
  title: string;
  version: string;
  type?: string;
  disable?: boolean;
}

export interface VersionCheck {
  success: boolean;
  cached: boolean;
  data?: any;
  error?: any;
  time: number;
}

export interface Stats {
  toJson: () => any;
}

export interface BuilderResult {
  totalTime?: ReturnType<typeof process.hrtime>;
  stats?: Stats;
}

export type PackageJson = PackageJsonFromTypeFest & Record<string | symbol, any>;

// TODO: This could be exported to the outside world and used in `options.ts` file of each `@storybook/APP`
// like it's described in docs/api/new-frameworks.md
export interface LoadOptions {
  pnp?: boolean;
  packageJson?: PackageJson;
  outputDir?: string;
  configDir?: string;
  cacheKey?: string;
  ignorePreview?: boolean;
  extendServer?: (server: HttpServer) => void;
}

export interface CLIBaseOptions {
  disableTelemetry?: boolean;
  enableCrashReports?: boolean;
  configDir?: string;
  loglevel?: string;
  quiet?: boolean;
}

export interface CLIOptions extends CLIBaseOptions {
  port?: number;
  ignorePreview?: boolean;
  previewUrl?: string;

Review comment (suggestion):
I don't think we should be changing the type for something that's purely internal. Just extend `PackageJsonWithDepsAndDevDeps` where needed.

Code after:
export interface Ref {
  id: string;
  url: string;
  title: string;
  version: string;
  type?: string;
  disable?: boolean;
}

export interface VersionCheck {
  success: boolean;
  cached: boolean;
  data?: any;
  error?: any;
  time: number;
}

export interface Stats {
  toJson: () => any;
}

export interface BuilderResult {
  totalTime?: ReturnType<typeof process.hrtime>;
  stats?: Stats;
}

export type PackageJson = PackageJsonFromTypeFest & Record<string, any>;

// TODO: This could be exported to the outside world and used in `options.ts` file of each `@storybook/APP`
// like it's described in docs/api/new-frameworks.md
export interface LoadOptions {
  pnp?: boolean;
  packageJson?: PackageJson;
  outputDir?: string;
  configDir?: string;
  cacheKey?: string;
  ignorePreview?: boolean;
  extendServer?: (server: HttpServer) => void;
}

export interface CLIBaseOptions {
  disableTelemetry?: boolean;
  enableCrashReports?: boolean;
  configDir?: string;
  loglevel?: LogLevel;
  logfile?: string | boolean;
  quiet?: boolean;
}

export interface CLIOptions extends CLIBaseOptions {
  port?: number;
Repository: storybookjs/storybook
File: code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.ts (TypeScript)

Code before:
    const content = await readFile(mainConfigPath, 'utf-8');

    const isESM = containsESMUsage(content);
    const isWithRequire = containsRequireUsage(content);
    const isWithBanner = hasRequireBanner(content);

    // Check if the file is ESM format based on content
    if (!isESM) {
      return null;
    }

    // Check if the file already has the require banner
    if (isWithBanner) {
      return null;
    }

    // Check if the file contains require usage
    if (!isWithRequire) {
      return null;
    }

    return { storybookVersion };
  },

  prompt() {
    return dedent`We have detected that your main config file is in ESM format but contains 'require' references. This will cause issues in Storybook 10. We'll add a compatibility banner to make it work.`;
  },

  async run({ dryRun, mainConfigPath }) {
    if (dryRun) {
      return;
    }

    const content = await readFile(mainConfigPath, 'utf-8');
    const banner = getRequireBanner();
    const comment = bannerComment;

    const newContent = [banner, comment, content].join('\n\n');

    await writeFile(mainConfigPath, newContent);
  },
} satisfies Fix<FixFauxEsmRequireRunOptions>;


Review comment (bug):
```suggestion
    return dedent`Main config is ESM but uses 'require'. This will break in Storybook 10; Adding compatibility banner`;
```

Making the prompt less verbose, since in the new upgrade flow, the prompt is shown as a checkbox item, and we want to keep the prompt as short as possible.

Code after:
    const isWithBanner = hasRequireBanner(content);

    // Check if the file is ESM format based on content
    if (!isESM) {
      return null;
    }

    // Check if the file already has the require banner
    if (isWithBanner) {
      return null;
    }

    // Check if the file contains require usage
    if (!isWithRequire) {
      return null;
    }

    return true;
  },

  prompt() {
    return dedent`Main config is ESM but uses 'require'. This will break in Storybook 10; Adding compatibility banner`;
  },

  async run({ dryRun, mainConfigPath }) {
    if (dryRun) {
      return;
    }

    const content = await readFile(mainConfigPath, 'utf-8');
    const banner = getRequireBanner();
    const comment = bannerComment;

    const newContent = [banner, comment, content].join('\n');

    await writeFile(mainConfigPath, newContent);
  },
} satisfies Fix<true>;
Repository: storybookjs/storybook
File: code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.ts (TypeScript)

Code before:
      return null;
    }

    // Read the raw file content to check for ESM syntax and require usage
    const content = await readFile(mainConfigPath, 'utf-8');

    const isESM = containsESMUsage(content);
    const isWithRequire = containsRequireUsage(content);
    const isWithBanner = hasRequireBanner(content);

    // Check if the file is ESM format based on content
    if (!isESM) {
      return null;
    }

    // Check if the file already has the require banner
    if (isWithBanner) {
      return null;
    }

    // Check if the file contains require usage
    if (!isWithRequire) {
      return null;
    }

    return { storybookVersion };
  },

  prompt() {
    return dedent`We have detected that your main config file is in ESM format but contains 'require' references. This will cause issues in Storybook 10. We'll add a compatibility banner to make it work.`;
  },

  async run({ dryRun, mainConfigPath }) {
    if (dryRun) {
      return;
    }

    const content = await readFile(mainConfigPath, 'utf-8');
    const banner = getRequireBanner();
    const comment = bannerComment;

    const newContent = [banner, comment, content].join('\n\n');

    await writeFile(mainConfigPath, newContent);
  },
} satisfies Fix<FixFauxEsmRequireRunOptions>;


Review comment (suggestion):
Returning `true` is enough, since `storybookVersion` isn't really used in the `run` function.

Code after:
    const content = await readFile(mainConfigPath, 'utf-8');

    const isESM = containsESMUsage(content);
    const isWithRequire = containsRequireUsage(content);
    const isWithBanner = hasRequireBanner(content);

    // Check if the file is ESM format based on content
    if (!isESM) {
      return null;
    }

    // Check if the file already has the require banner
    if (isWithBanner) {
      return null;
    }

    // Check if the file contains require usage
    if (!isWithRequire) {
      return null;
    }

    return true;
  },

  prompt() {
    return dedent`Main config is ESM but uses 'require'. This will break in Storybook 10; Adding compatibility banner`;
  },

  async run({ dryRun, mainConfigPath }) {
    if (dryRun) {
      return;
    }

    const content = await readFile(mainConfigPath, 'utf-8');
    const banner = getRequireBanner();
    const comment = bannerComment;

    const newContent = [banner, comment, content].join('\n');

    await writeFile(mainConfigPath, newContent);
  },
} satisfies Fix<true>;
Repository: storybookjs/storybook
File: scripts/get-template.ts (TypeScript)

Code before:
    }
    await checkParallelism(cadence as Cadence, task, fix);
    return;
  }

  if (!cadence) {
    throw new Error('Need to supply cadence to get template script');
  }

  const { CIRCLE_NODE_INDEX = 0, CIRCLE_NODE_TOTAL = 1 } = process.env;

  console.log(
    await getTemplate(cadence as Cadence, task, {
      index: +CIRCLE_NODE_INDEX,
      total: +CIRCLE_NODE_TOTAL,
    })
  );
}

if (esMain(import.meta.url)) {
  program
    .description('Retrieve the template to run for a given cadence and task')
    .option('--cadence <cadence>', 'Which cadence you want to run the script for')
    .option('--task <task>', 'Which task you want to run the script for')
    .option('--check', 'Throws an error when the parallelism counts for tasks are incorrect', false)
    .option('--fix', 'Automatically fix parallelism counts in .circleci/config.yml', false);

  program.parse(process.argv);

  const options = program.opts() as RunOptions;

  run(options).catch((err) => {
    console.error(err);
    process.exit(1);
  });
}


Review comment (suggestion):
```suggestion
    .option('--fix', 'Automatically fix parallelism counts in workflow files', false);
```

Code after:
    }
    await checkParallelism(cadence as Cadence, task, fix);
    return;
  }

  if (!cadence) {
    throw new Error('Need to supply cadence to get template script');
  }

  const { CIRCLE_NODE_INDEX = 0, CIRCLE_NODE_TOTAL = 1 } = process.env;

  console.log(
    await getTemplate(cadence as Cadence, task, {
      index: +CIRCLE_NODE_INDEX,
      total: +CIRCLE_NODE_TOTAL,
    })
  );
}

if (esMain(import.meta.url)) {
  program
    .description('Retrieve the template to run for a given cadence and task')
    .option('--cadence <cadence>', 'Which cadence you want to run the script for')
    .option('--task <task>', 'Which task you want to run the script for')
    .option('--check', 'Throws an error when the parallelism counts for tasks are incorrect', false)
    .option('--fix', 'Automatically fix parallelism counts in workflow files', false);

  program.parse(process.argv);

  const options = program.opts() as RunOptions;

  run(options).catch((err) => {
    console.error(err);
    process.exit(1);
  });
}
Repository: storybookjs/storybook
File: code/lib/cli-storybook/src/codemod/helpers/remove-unused-types.test.ts (TypeScript)

Code before:
      -       story: StoryObj<typeof Button>;
      -     }
      -   }
      - };
      - interface UnusedInterface extends Meta {}
      - interface UnusedDeepInterface {
      -   baz: {
      -     qux: {
      -       meta: Meta<typeof Button>;
      -     }
      -   }
      - };
      - 
      - 
        export default { component: Button };
    `);
  });

  it('should not remove used Storybook types', async () => {
    const source = dedent`
      // Nothing in this file should be removed or modified
      import { StoryFn, StoryObj, ComponentStory, Meta, MetaObj, ComponentMeta } from '@storybook/react';
      import { Button } from './Button';

      type Alias = StoryFn<typeof Button>;
      type Alias2 = Alias & { b: string };
      type Story = StoryObj & { a: string };
      type DeepType = {
        foo: {
          bar: {
            story: ComponentStory<typeof Button>;
          }
        }
      };
      interface Interface extends Meta {}
      interface DeepInterface {
        baz: {
          qux: {
            meta: MetaObj<typeof Button>;
          }
        }
      };
      const X: ComponentMeta = {}

      function foo(a: Story, c: DeepType, d: Interface, e: DeepInterface){}

      export default {};
    `;

    const transformed = getTransformed(source);


Review comment (suggestion):
```suggestion
      import type { StoryFn, StoryObj, ComponentStory, Meta, MetaObj, ComponentMeta } from '@storybook/react';
```

Code after:
      - type UnusedAlias4 = ComponentMeta<typeof Button>;
      - type UnusedDeepType = {
      -   foo: {
      -     bar: {
      -       story: StoryObj<typeof Button>;
      -     }
      -   }
      - };
      - interface UnusedInterface extends Meta {}
      - interface UnusedDeepInterface {
      -   baz: {
      -     qux: {
      -       meta: Meta<typeof Button>;
      -     }
      -   }
      - };
      - 
      - 
        export default { component: Button };
    `);
  });

  it('should not remove used Storybook types', async () => {
    const source = dedent`
      // Nothing in this file should be removed or modified
      import { StoryFn, StoryObj, ComponentStory, Meta, MetaObj, ComponentMeta } from '@storybook/react';
      import { Button } from './Button';

      type Alias = StoryFn<typeof Button>;
      type Alias2 = Alias & { b: string };
      type Story = StoryObj & { a: string };
      type DeepType = {
        foo: {
          bar: {
            story: ComponentStory<typeof Button>;
          }
        }
      };
      interface Interface extends Meta {}
      interface DeepInterface {
        baz: {
          qux: {
            meta: MetaObj<typeof Button>;
          }
        }
      };
      const X: ComponentMeta = {}

      function foo(a: Story, c: DeepType, d: Interface, e: DeepInterface){}

      export default {};
Repository: storybookjs/storybook
File: code/core/src/components/components/Form/Radio.tsx (TypeScript)

Code before:
import React from 'react';

import { color, styled } from 'storybook/theming';

const Input = styled.input(({ theme }) => ({
  appearance: 'none',
  backgroundColor: theme.input.background,
  border: `1px solid ${theme.base == 'dark' ? 'hsl(0 0 100 / 0.4)' : 'hsl(0 0 0 / 0.44)'}`,
  borderRadius: 8,
  display: 'grid',
  flexShrink: 0,
  height: 16,
  margin: -1,
  placeContent: 'center',
  transition: 'background-color 0.1s',
  width: 16,

  '&:enabled': {
    cursor: 'pointer',
  },
  '&:disabled': {
    backgroundColor: 'transparent',
    borderColor: theme.input.border,
  },
  '&:disabled:checked': {
    backgroundColor: theme.base === 'dark' ? color.dark : theme.color.mediumdark,
    borderColor: theme.base === 'dark' ? color.dark : theme.color.mediumdark,
  },
  '&:checked': {
    backgroundColor: color.secondary,
    borderColor: color.secondary,
    boxShadow: `inset 0 0 0 2px ${theme.input.background}`,
  },

Review comment (suggestion):
```suggestion
  border: `1px solid ${theme.base === 'dark' ? 'hsl(0 0 100 / 0.4)' : 'hsl(0 0 0 / 0.44)'}`,
```

Here too, or ESLint will gnaw away at the soul of the next developer to open the file.

Code after:
import React from 'react';

import { color, styled } from 'storybook/theming';

const Input = styled.input(({ theme }) => ({
  appearance: 'none',
  backgroundColor: theme.input.background,
  border: `1px solid ${theme.base === 'dark' ? 'hsl(0 0 100 / 0.4)' : 'hsl(0 0 0 / 0.44)'}`,
  borderRadius: 8,
  display: 'grid',
  flexShrink: 0,
  height: 16,
  margin: -1,
  placeContent: 'center',
  transition: 'background-color 0.1s',
  width: 16,

  '&:enabled': {
    cursor: 'pointer',
  },
  '&:disabled': {
    backgroundColor: 'transparent',
    borderColor: theme.input.border,
  },
  '&:disabled:checked': {
    backgroundColor: theme.base === 'dark' ? color.dark : theme.color.mediumdark,
    borderColor: theme.base === 'dark' ? color.dark : theme.color.mediumdark,
  },
  '&:checked': {
    backgroundColor: color.secondary,
    borderColor: color.secondary,
    boxShadow: `inset 0 0 0 2px ${theme.input.background}`,
  },
Repository: storybookjs/storybook
File: code/renderers/react/src/componentManifest/valid-package-name.ts (TypeScript)

Code before:

  // mIxeD CaSe nAMEs
  if (name.toLowerCase() !== name) {
    return false;
  }

  if (/[~'!()*]/.test(name.split('/').slice(-1)[0])) {
    return false;
  }

  if (encodeURIComponent(name) !== name) {
    const nameMatch = name.match(scopedPackagePattern);
    if (nameMatch) {
      const user = nameMatch[1];
      const pkg = nameMatch[2];

      if (pkg.startsWith('.')) {
        return false;
      }

      if (encodeURIComponent(user) === user && encodeURIComponent(pkg) === pkg) {
        return true;
      }
    }
    return false;
  }

  return false;
}


Review comment (suggestion):
```suggestion
  if (encodeURIComponent(name) !== name) {
    const nameMatch = name.match(scopedPackagePattern);
    if (nameMatch) {
      const org = nameMatch[1];
      const pkg = nameMatch[2];

      if (pkg.startsWith('.')) {
        return false;
      }

      if (encodeURIComponent(org) === org && encodeURIComponent(pkg) === pkg) {
        return true;
      }
    }
    return false;
  }
```

Code after:

  // mIxeD CaSe nAMEs
  if (name.toLowerCase() !== name) {
    return false;
  }

  if (/[~'!()*]/.test(name.split('/').slice(-1)[0])) {
    return false;
  }

  if (encodeURIComponent(name) !== name) {
    const nameMatch = name.match(scopedPackagePattern);
    if (nameMatch) {
      const org = nameMatch[1];
      const pkg = nameMatch[2];

      if (pkg.startsWith('.')) {
        return false;
      }

      if (encodeURIComponent(org) === org && encodeURIComponent(pkg) === pkg) {
        return true;
      }
    }
    return false;
  }

  return true;
}
Repository: storybookjs/storybook
File: code/core/src/theming/create.ts (TypeScript)

Code before:
const themesBase: Omit<Themes, 'normal'> = {
  light: lightThemeVars,
  dark: darkThemeVars,
};

const preferredColorScheme = getPreferredColorScheme();

export const themes: Themes = {
  ...themesBase,
  normal: themesBase[preferredColorScheme],
};

interface Rest {
  [key: string]: unknown;
}

export const create = (
  vars: ThemeVarsPartial = {
    base: preferredColorScheme,
  },
  rest?: Rest
): ThemeVars => {
  const base = themes[vars.base] ? vars.base : preferredColorScheme;
  const inherit: ThemeVars = {
    ...themes[preferredColorScheme],
    ...themes[vars.base],
    ...vars,
    base,
  };
  return {
    ...rest,
    ...inherit,
    barSelectedColor: vars.barSelectedColor || inherit.colorSecondary,
  };
};


Review comment (suggestion):
```suggestion
  ...(themes[vars.base] || {}),
```

**If themes[vars.base] is not defined**:
* We spread nothing here
* We assign `base` to `preferredColorScheme`

**If themes[vars.base] is define defined**:
* We spread that base theme inside our theme
* We keep track of the base theme through `base`

What you changed on line 37 is fine, the issue was with removing the `|| {}` that handles the null check when spreading the base theme.

The AI spotted that there is a logic regression, but misunderstood your intent so its suggestion isn't helpful.

Code after:
const themesBase: Omit<Themes, 'normal'> = {
  light: lightThemeVars,
  dark: darkThemeVars,
};

const preferredColorScheme = getPreferredColorScheme();

export const themes: Themes = {
  ...themesBase,
  normal: themesBase[preferredColorScheme],
};

interface Rest {
  [key: string]: unknown;
}

export const create = (
  vars: ThemeVarsPartial = {
    base: preferredColorScheme,
  },
  rest?: Rest
): ThemeVars => {
  const inherit: ThemeVars = {
    // We always inherit the preferred color scheme.
    ...themes[preferredColorScheme],
    // And then the declared theme base if it exists.
    ...(themes[vars.base] || {}),
    // And then the actual theme content.
    ...vars,
    // If no theme base was declared, we declare the preferred color scheme as the base.
    base: themes[vars.base] ? vars.base : preferredColorScheme,
  };
  return {
    ...rest,
    ...inherit,
    barSelectedColor: vars.barSelectedColor || inherit.colorSecondary,
  };
};
Repository: storybookjs/storybook
File: code/core/src/manager/components/sidebar/Sidebar.stories.tsx (TypeScript)

Code before:
    ...refs.optimized,
    // @ts-expect-error (non strict)
    filteredIndex: undefined as IndexHash,
    indexError,
  },
};

const refsEmpty = {
  optimized: {
    ...refs.optimized,
    // type: 'auto-inject',
    filteredIndex: {} as IndexHash,
  },
};

const waitForChecklistWidget = () =>
  waitFor(
    () =>
      expect(document.getElementById('storybook-checklist-widget')?.checkVisibility()).toBe(true),
    { timeout: 5000 }
  );

export const Simple: Story = {
  play: waitForChecklistWidget,
};

export const SimpleInProduction: Story = {
  args: {
    showCreateStoryButton: false,
  },
  beforeEach: () => {
    const configType = global.CONFIG_TYPE;
    global.CONFIG_TYPE = 'PRODUCTION';
    return () => {
      global.CONFIG_TYPE = configType;
    };
  },
};

export const Mobile: Story = {
  decorators: [mobileLayoutDecorator],
  globals: { sb_theme: 'light', viewport: { value: 'mobile1' } },
  play: waitForChecklistWidget,
};

export const Loading: Story = {
  args: {
    previewInitialized: false,
    index: undefined,
  },
};

Review comment (suggestion):
As this is needed in every story, you could write a play function in the meta level. Then for the stories that overwrite the play function, they can do this:
```
  play: async (context) => {
    await meta.play(context)
    // ...
  },
```

Code after:
    ...refs.optimized,
    // @ts-expect-error (non strict)
    filteredIndex: undefined as IndexHash,
    indexError,
  },
};

const refsEmpty = {
  optimized: {
    ...refs.optimized,
    // type: 'auto-inject',
    filteredIndex: {} as IndexHash,
  },
};

const waitForChecklistWidget = async () => {
  await waitFor(
    () =>
      expect(document.getElementById('storybook-checklist-widget')?.checkVisibility()).toBe(true),
    { timeout: 5000 }
  );
  await wait(300); // wait for expand animation
};

export const Simple: Story = {
  play: waitForChecklistWidget,
};

export const SimpleInProduction: Story = {
  args: {
    showCreateStoryButton: false,
  },
  beforeEach: () => {
    const configType = global.CONFIG_TYPE;
    global.CONFIG_TYPE = 'PRODUCTION';
    return () => {
      global.CONFIG_TYPE = configType;
    };
  },
};

export const Mobile: Story = {
  decorators: [mobileLayoutDecorator],
  globals: { sb_theme: 'light', viewport: { value: 'mobile1' } },
  play: waitForChecklistWidget,
};

export const Loading: Story = {
  args: {
    previewInitialized: false,
    index: undefined,