ronantakizawa/codereview-bench huggingface dataset:
Repository: evanw/esbuild
File: internal/bundler/bundler_test.go (Go)

Code before:
				class Foo {
					#foo
					foo = class {
						#foo
						#foo2
						#bar
					}
					get #bar() {}
					set #bar() {}
				}
				class Bar {
					#foo
					foo = class {
						#foo2
						#foo
						#bar
					}
					get #bar() {}
					set #bar() {}
				}
			`,
		},
		entryPaths: []string{"/entry.js"},
		parseOptions: parser.ParseOptions{
			IsBundling: false,
		},
		bundleOptions: BundleOptions{
			IsBundling:        false,
			MinifyIdentifiers: true,
			AbsOutputFile:     "/out.js",
		},
		expected: map[string]string{
			"/out.js": `class Foo {
  #a;
  foo = class {
    #c;
    #d;
    #e;
  };
  get #b() {
  }
  set #b() {
  }
}
class Bar {
  #a;
  foo = class {
    #c;
    #d;
    #e;
  };

Review comment (bug):
@evanw as a side question, could you tell please why this is bundled with suffix, I expected that it would be direct variable, should it be considered as a bug?

Code after:
				import { DocumentClient } from 'aws-sdk/clients/dynamodb';
				export const s3 = new S3();
				export const dynamodb = new DocumentClient();
			`,
		},
		entryPaths: []string{"/index.js"},
		parseOptions: parser.ParseOptions{
			IsBundling: true,
		},
		bundleOptions: BundleOptions{
			IsBundling:    true,
			AbsOutputFile: "/out.js",
		},
		resolveOptions: resolver.ResolveOptions{
			ExternalModules: map[string]bool{
				"aws-sdk": true,
			},
		},
		expected: map[string]string{
			"/out.js": `// /index.js
import {S3} from "aws-sdk";
import {DocumentClient} from "aws-sdk/clients/dynamodb";
const s3 = new S3();
const dynamodb2 = new DocumentClient();
export {
  dynamodb2 as dynamodb,
  s3
};
`,
		},
	})
}

// This test case makes sure many entry points don't cause a crash
func TestManyEntryPoints(t *testing.T) {
	expectBundled(t, bundled{
		files: map[string]string{
			"/shared.js": `export default 123`,

			"/e00.js": `import x from './shared'; console.log(x)`,
			"/e01.js": `import x from './shared'; console.log(x)`,
			"/e02.js": `import x from './shared'; console.log(x)`,
			"/e03.js": `import x from './shared'; console.log(x)`,
			"/e04.js": `import x from './shared'; console.log(x)`,
			"/e05.js": `import x from './shared'; console.log(x)`,
			"/e06.js": `import x from './shared'; console.log(x)`,
			"/e07.js": `import x from './shared'; console.log(x)`,
			"/e08.js": `import x from './shared'; console.log(x)`,
			"/e09.js": `import x from './shared'; console.log(x)`,

			"/e10.js": `import x from './shared'; console.log(x)`,
Repository: evanw/esbuild
File: lib/shared/types.ts (TypeScript)

Code before:
  kind: ImportKind;
  pluginData: any;
}

export type ImportKind =
  | 'entry-point'

  // JS
  | 'import-statement'
  | 'require-call'
  | 'dynamic-import'
  | 'require-resolve'

  // CSS
  | 'import-rule'
  | 'url-token'

export interface OnResolveResult {
  pluginName?: string;

  errors?: PartialMessage[];
  warnings?: PartialMessage[];

  path?: string;
  external?: boolean;
  sideEffectFree?: boolean;
  namespace?: string;
  pluginData?: any;

  watchFiles?: string[];
  watchDirs?: string[];
}

export interface OnLoadOptions {
  filter: RegExp;
  namespace?: string;
}

export interface OnLoadArgs {
  path: string;
  namespace: string;
  pluginData: any;
}

export interface OnLoadResult {
  pluginName?: string;

  errors?: PartialMessage[];
  warnings?: PartialMessage[];

  contents?: string | Uint8Array;

Review comment (suggestion):
I'm going to go with `sideEffects: false` instead of `sideEffectFree: true` to match Webpack. I think this will make the feature more intuitive.


Code after:
  kind: ImportKind;
  pluginData: any;
}

export type ImportKind =
  | 'entry-point'

  // JS
  | 'import-statement'
  | 'require-call'
  | 'dynamic-import'
  | 'require-resolve'

  // CSS
  | 'import-rule'
  | 'url-token'

export interface OnResolveResult {
  pluginName?: string;

  errors?: PartialMessage[];
  warnings?: PartialMessage[];

  path?: string;
  external?: boolean;
  sideEffects?: boolean;
  namespace?: string;
  pluginData?: any;

  watchFiles?: string[];
  watchDirs?: string[];
}

export interface OnLoadOptions {
  filter: RegExp;
  namespace?: string;
}

export interface OnLoadArgs {
  path: string;
  namespace: string;
  pluginData: any;
}

export interface OnLoadResult {
  pluginName?: string;

  errors?: PartialMessage[];
  warnings?: PartialMessage[];

  contents?: string | Uint8Array;
Repository: evanw/esbuild
File: lib/shared/types.ts (TypeScript)

Code before:
   * You can use this as an alternative to "wasmURL" for environments where it's
   * not possible to download the WebAssembly module.
   */
  wasmModule?: WebAssembly.Module

  /**
   * By default esbuild runs the WebAssembly-based browser API in a web worker
   * to avoid blocking the UI thread. This can be disabled by setting "worker"
   * to false.
   */
  worker?: boolean
}

export let version: string

// Call this function to terminate esbuild's child process. The child process
// is not terminated and re-created for each API call because it's more
// efficient to keep it around when there are multiple API calls.
//
// In node this happens automatically before the parent node process exits. So
// you only need to call this if you know you will not make any more esbuild
// API calls and you want to clean up resources.
//
// Unlike node, Deno lacks the necessary APIs to clean up child processes
// automatically. You must manually call stop() in Deno when you're done
// using esbuild or Deno will continue running forever.
export declare function stop(): Promise<void>


Review comment (suggestion):
Also FYI, the new API has `Deno.ChildProcess#ref` and `Deno.ChildProcess#unref` methods that should let most users avoid calling this API (unless they are running in a test with sanitization, like you are here).

I suggest you call `process.unref()` like you do in Node, and then most users don't have to call this method at all anymore :)

Code after:
   * You can use this as an alternative to "wasmURL" for environments where it's
   * not possible to download the WebAssembly module.
   */
  wasmModule?: WebAssembly.Module

  /**
   * By default esbuild runs the WebAssembly-based browser API in a web worker
   * to avoid blocking the UI thread. This can be disabled by setting "worker"
   * to false.
   */
  worker?: boolean
}

export let version: string

// Call this function to terminate esbuild's child process. The child process
// is not terminated and re-created after each API call because it's more
// efficient to keep it around when there are multiple API calls. This child
// process normally exits automatically when the parent process exits, so you
// usually don't need to call this function.
//
// One reason you might want to call this is if you know you will not make any
// more esbuild API calls and you want to clean up resources (since the esbuild
// child process takes up some memory even when idle).
//
// Another reason you might want to call this is if you are using esbuild from
// within a Deno test. Deno fails tests that create a child process without
// killing it before the test ends, so you have to call this function (and
// await the returned promise) in every Deno test that uses esbuild.
export declare function stop(): Promise<void>
Repository: excalidraw/excalidraw
File: src/base64.ts (TypeScript)

Code before:
// `btoa(unescape(encodeURIComponent(str)))` hack doesn't work in edge cases and
//  `unescape` API shouldn't be used anyway.
// This implem is ~10x faster than using fromCharCode in a loop (in Chrome).
const stringToByteString = (str: string): Promise<string> => {
  return new Promise((resolve, reject) => {
    const blob = new Blob([new TextEncoder().encode(str)]);
    const reader = new FileReader();
    reader.onload = function (event) {
      if (!event.target || typeof event.target.result !== "string") {
        return reject(new Error("couldn't convert to byte string"));
      }
      resolve(event.target.result);
    };
    reader.readAsBinaryString(blob);
  });
};

function byteStringToArrayBuffer(byteString: string) {
  const buffer = new ArrayBuffer(byteString.length);
  const bufferView = new Uint8Array(buffer);
  for (let i = 0, len = byteString.length; i < len; i++) {
    bufferView[i] = byteString.charCodeAt(i);
  }
  return buffer;
}

const byteStringToString = (byteString: string) => {

Review comment (security):
```suggestion
// `unescape` API shouldn't be used anyway.
```

Code after:
// `btoa(unescape(encodeURIComponent(str)))` hack doesn't work in edge cases and
// `unescape` API shouldn't be used anyway.
// This implem is ~10x faster than using fromCharCode in a loop (in Chrome).
const stringToByteString = (str: string): Promise<string> => {
  return new Promise((resolve, reject) => {
    const blob = new Blob([new TextEncoder().encode(str)]);
    const reader = new FileReader();
    reader.onload = function (event) {
      if (!event.target || typeof event.target.result !== "string") {
        return reject(new Error("couldn't convert to byte string"));
      }
      resolve(event.target.result);
    };
    reader.readAsBinaryString(blob);
  });
};

function byteStringToArrayBuffer(byteString: string) {
  const buffer = new ArrayBuffer(byteString.length);
  const bufferView = new Uint8Array(buffer);
  for (let i = 0, len = byteString.length; i < len; i++) {
    bufferView[i] = byteString.charCodeAt(i);
  }
  return buffer;
}

const byteStringToString = (byteString: string) => {
Repository: excalidraw/excalidraw
File: src/components/Stack.tsx (TypeScript)

Code before:
import "./Stack.scss";

import React from "react";
import clsx from "clsx";

type StackProps = {
  children: React.ReactNode;
  gap?: number;
  align?: "start" | "center" | "end" | "baseline";
  justifyContent?: "center" | "space-around" | "space-between";
  className?: string | boolean;
  style?: object;
};

const RowStack = ({
  children,
  gap,
  align,
  justifyContent,
  className,
  style,
}: StackProps) => {
  return (
    <div
      className={clsx("Stack Stack_horizontal", className)}
      style={
        {
          "--gap": gap,
          alignItems: align,
          justifyContent,
          ...style,
        } as React.CSSProperties
      }
    >
      {children}
    </div>
  );

Review comment (suggestion):
```suggestion
  style?: React.CSSProperties;
```
Might be better to narrow type this.

Code after:
import "./Stack.scss";

import React from "react";
import clsx from "clsx";

type StackProps = {
  children: React.ReactNode;
  gap?: number;
  align?: "start" | "center" | "end" | "baseline";
  justifyContent?: "center" | "space-around" | "space-between";
  className?: string | boolean;
  style?: React.CSSProperties;
};

const RowStack = ({
  children,
  gap,
  align,
  justifyContent,
  className,
  style,
}: StackProps) => {
  return (
    <div
      className={clsx("Stack Stack_horizontal", className)}
      style={{
        "--gap": gap,
        alignItems: align,
        justifyContent,
        ...style,
      }}
    >
      {children}
    </div>
  );
};
Repository: excalidraw/excalidraw
File: src/constants.ts (TypeScript)

Code before:
export const STORAGE_KEYS = {
  LOCAL_STORAGE_LIBRARY: "excalidraw-library",
};

// time in milliseconds
export const TAP_TWICE_TIMEOUT = 300;
export const TOUCH_CTX_MENU_TIMEOUT = 500;
export const TITLE_TIMEOUT = 10000;
export const TOAST_TIMEOUT = 5000;
export const VERSION_TIMEOUT = 30000;
export const SCROLL_TIMEOUT = 500;

export const ZOOM_STEP = 0.1;

// Report a user inactive after IDLE_THRESHOLD milliseconds
export const IDLE_THRESHOLD = 60_000;
// Report a user active each ACTIVE_THRESHOLD milliseconds
export const ACTIVE_THRESHOLD = 3_000;

export const MODES = {
  VIEW: "viewMode",
  ZEN: "zenMode",
  GRID: "gridMode",
};

export const APPEARANCE_FILTER = "invert(93%) hue-rotate(180deg)";


Review comment (suggestion):
```suggestion

// Should be in sync with value in theme.scss
```

Code after:

export const STORAGE_KEYS = {
  LOCAL_STORAGE_LIBRARY: "excalidraw-library",
};

// time in milliseconds
export const TAP_TWICE_TIMEOUT = 300;
export const TOUCH_CTX_MENU_TIMEOUT = 500;
export const TITLE_TIMEOUT = 10000;
export const TOAST_TIMEOUT = 5000;
export const VERSION_TIMEOUT = 30000;
export const SCROLL_TIMEOUT = 500;

export const ZOOM_STEP = 0.1;

// Report a user inactive after IDLE_THRESHOLD milliseconds
export const IDLE_THRESHOLD = 60_000;
// Report a user active each ACTIVE_THRESHOLD milliseconds
export const ACTIVE_THRESHOLD = 3_000;

export const MODES = {
  VIEW: "viewMode",
  ZEN: "zenMode",
  GRID: "gridMode",
};

export const APPEARANCE_FILTER = cssVariables.appearanceFilter;
Repository: excalidraw/excalidraw
File: src/actions/actionExport.tsx (TypeScript)

Code before:
      title={t("buttons.load")}
      aria-label={t("buttons.load")}
      showAriaLabel={useIsMobile()}
      onClick={updateData}
    />
  ),
});

export const actionExportWithDarkMode = register({
  name: "exportWithDarkMode",
  perform: (_elements, appState, value) => {
    return {
      appState: { ...appState, exportWithDarkMode: value },
      commitToHistory: false,
    };
  },
  PanelComponent: ({ appState, updateData }) => (
    <div
      style={{
        display: "flex",
        justifyContent: "flex-end",
        marginTop: "-45px",
        marginBottom: "10px",
      }}
    >
      <DarkModeToggle
        value={appState.exportWithDarkMode ? "dark" : "light"}
        onChange={(appearance: Appearence) => {
          updateData(appearance === "dark");
        }}
      />
    </div>
  ),
});


Review comment (suggestion):
Let's make the `DarkModeToggle` component take `props.title` so we can supply a different tooltip for the export.

Code after:
      title={t("buttons.load")}
      aria-label={t("buttons.load")}
      showAriaLabel={useIsMobile()}
      onClick={updateData}
    />
  ),
});

export const actionExportWithDarkMode = register({
  name: "exportWithDarkMode",
  perform: (_elements, appState, value) => {
    return {
      appState: { ...appState, exportWithDarkMode: value },
      commitToHistory: false,
    };
  },
  PanelComponent: ({ appState, updateData }) => (
    <div
      style={{
        display: "flex",
        justifyContent: "flex-end",
        marginTop: "-45px",
        marginBottom: "10px",
      }}
    >
      <DarkModeToggle
        value={appState.exportWithDarkMode ? "dark" : "light"}
        onChange={(appearance: Appearence) => {
          updateData(appearance === "dark");
        }}
        title={t("labels.toggleExportColorScheme")}
      />
    </div>
  ),
});
Repository: excalidraw/excalidraw
File: src/data/blob.ts (TypeScript)

Code before:
};

export const loadFromBlob = async (
  blob: Blob,
  /** @see restore.localAppState */
  localAppState: AppState | null,
) => {
  const contents = await parseFileContents(blob);
  try {
    const data = JSON.parse(contents);
    if (!isValidExcalidrawData(data)) {
      throw new Error(t("alerts.couldNotLoadInvalidFile"));
    }
    const result = restore(
      {
        elements: clearElementsForExport(data.elements || []),
        appState: {
          theme: localAppState?.theme,
          fileHandle: blob.handle ?? null,
          ...cleanAppStateForExport(data.appState || {}),
          ...(localAppState
            ? calculateScrollCenter(data.elements || [], localAppState, null)
            : {}),
        },
      },
      localAppState,
    );

    return result;
  } catch (error) {
    console.error(error.message);
    throw new Error(t("alerts.couldNotLoadInvalidFile"));
  }
};

export const loadLibraryFromBlob = async (blob: Blob) => {
  const contents = await parseFileContents(blob);
  const data: LibraryData = JSON.parse(contents);
  if (data.type !== "excalidrawlib") {
    throw new Error(t("alerts.couldNotLoadInvalidFile"));
  }
  return data;
};

export const canvasToBlob = async (
  canvas: HTMLCanvasElement,
): Promise<Blob> => {
  return new Promise((resolve, reject) => {
    try {
      canvas.toBlob((blob) => {
        if (!blob) {

Review comment (suggestion):
I've removed this to allow for importing files without an extension. We're doing a check above to see if the file contains an appropriate JSON so we should be fine.

Code after:
  return "";
};

export const loadFromBlob = async (
  blob: Blob,
  /** @see restore.localAppState */
  localAppState: AppState | null,
) => {
  const contents = await parseFileContents(blob);
  try {
    const data = JSON.parse(contents);
    if (!isValidExcalidrawData(data)) {
      throw new Error(t("alerts.couldNotLoadInvalidFile"));
    }
    const result = restore(
      {
        elements: clearElementsForExport(data.elements || []),
        appState: {
          theme: localAppState?.theme,
          fileHandle: blob.handle ?? null,
          ...cleanAppStateForExport(data.appState || {}),
          ...(localAppState
            ? calculateScrollCenter(data.elements || [], localAppState, null)
            : {}),
        },
      },
      localAppState,
    );

    return result;
  } catch (error) {
    console.error(error.message);
    throw new Error(t("alerts.couldNotLoadInvalidFile"));
  }
};

export const loadLibraryFromBlob = async (blob: Blob) => {
  const contents = await parseFileContents(blob);
  const data: LibraryData = JSON.parse(contents);
  if (data.type !== EXPORT_DATA_TYPES.excalidrawLibrary) {
    throw new Error(t("alerts.couldNotLoadInvalidFile"));
  }
  return data;
};

export const canvasToBlob = async (
  canvas: HTMLCanvasElement,
): Promise<Blob> => {
  return new Promise((resolve, reject) => {
    try {
      canvas.toBlob((blob) => {
Repository: excalidraw/excalidraw
File: src/components/App.tsx (TypeScript)

Code before:
        } catch (error) {
          console.warn(error.name, error.message);
        }
      }
      await this.loadFileToCanvas(file);
    }
  };

  loadFileToCanvas = (file: Blob) => {
    loadFromBlob(file, this.state)
      .then(({ elements, appState }) =>
        this.syncActionResult({
          elements,
          appState: {
            ...(appState || this.state),
            isLoading: false,
          },
          commitToHistory: true,
        }),
      )
      .catch((error) => {
        this.setState({ isLoading: false, errorMessage: error.message });
      });
  };

  private handleCanvasContextMenu = (
    event: React.PointerEvent<HTMLCanvasElement>,
  ) => {
    event.preventDefault();
    this.openContextMenu(event);
  };

  private maybeDragNewGenericElement = (
    pointerDownState: PointerDownState,
    event: MouseEvent | KeyboardEvent,
  ): void => {
    const draggingElement = this.state.draggingElement;
    const pointerCoords = pointerDownState.lastCoords;
    if (!draggingElement) {
      return;
    }
    if (draggingElement.type === "selection") {
      dragNewElement(
        draggingElement,
        this.state.elementType,
        pointerDownState.origin.x,
        pointerDownState.origin.y,
        pointerCoords.x,
        pointerCoords.y,
        distance(pointerDownState.origin.x, pointerCoords.x),
        distance(pointerDownState.origin.y, pointerCoords.y),

Review comment (suggestion):
We should probably move the allowed extensions into `constants.ts`.

Code after:
          },
          commitToHistory: true,
        });
        return;
      }
    } catch (error) {
      return this.setState({
        isLoading: false,
        errorMessage: error.message,
      });
    }

    const libraryShapes = event.dataTransfer.getData(MIME_TYPES.excalidrawlib);
    if (libraryShapes !== "") {
      this.addElementsFromPasteOrLibrary(
        JSON.parse(libraryShapes),
        event.clientX,
        event.clientY,
      );
      return;
    }

    const file = event.dataTransfer?.files[0];
    if (
      file?.type === MIME_TYPES.excalidrawlib ||
      file?.name?.endsWith(".excalidrawlib")
    ) {
      Library.importLibrary(file)
        .then(() => {
          this.setState({ isLibraryOpen: false });
        })
        .catch((error) =>
          this.setState({ isLoading: false, errorMessage: error.message }),
        );
      // default: assume an Excalidraw file regardless of extension/MimeType
    } else {
      this.setState({ isLoading: true });
      if (supported) {
        try {
          // This will only work as of Chrome 86,
          // but can be safely ignored on older releases.
          const item = event.dataTransfer.items[0];
          (file as any).handle = await (item as any).getAsFileSystemHandle();
        } catch (error) {
          console.warn(error.name, error.message);
        }
      }
      await this.loadFileToCanvas(file);
    }
  };
Repository: excalidraw/excalidraw
File: src/excalidraw-app/collab/RoomDialog.tsx (TypeScript)

Code before:
} from "../../components/icons";
import { ToolButton } from "../../components/ToolButton";
import { t } from "../../i18n";
import "./RoomDialog.scss";
import Stack from "../../components/Stack";

const RoomDialog = ({
  handleClose,
  activeRoomLink,
  username,
  onUsernameChange,
  onRoomCreate,
  onRoomDestroy,
  setErrorMessage,
}: {
  handleClose: () => void;
  activeRoomLink: string;
  username: string;
  onUsernameChange: (username: string) => void;
  onRoomCreate: () => void;
  onRoomDestroy: () => void;
  setErrorMessage: (message: string) => void;
}) => {
  const roomLinkInput = useRef<HTMLInputElement>(null);

  const getShareIcon = () => {
    const navigator = window.navigator as any;
    const isAppleBrowser = /Apple/.test(navigator.vendor);
    const isWindowsBrowser = navigator.appVersion.indexOf("Win") !== -1;

    if (isAppleBrowser) {
      return shareIOS;
    } else if (isWindowsBrowser) {
      return shareWindows;
    }

    return share;
  };

  const copyRoomLink = async () => {
    try {
      await copyTextToSystemClipboard(activeRoomLink);
    } catch (error) {
      setErrorMessage(error.message);
    }
    if (roomLinkInput.current) {
      roomLinkInput.current.select();
    }
  };

  const shareRoomLink = async () => {

Review comment (suggestion):
since it is a pure function lets keep it outside`RoomDialog`  so its not recreated (new reference) for every rerender.

Code after:
} from "../../components/icons";
import { ToolButton } from "../../components/ToolButton";
import { t } from "../../i18n";
import "./RoomDialog.scss";
import Stack from "../../components/Stack";

const getShareIcon = () => {
  const navigator = window.navigator as any;
  const isAppleBrowser = /Apple/.test(navigator.vendor);
  const isWindowsBrowser = navigator.appVersion.indexOf("Win") !== -1;

  if (isAppleBrowser) {
    return shareIOS;
  } else if (isWindowsBrowser) {
    return shareWindows;
  }

  return share;
};

const RoomDialog = ({
  handleClose,
  activeRoomLink,
  username,
  onUsernameChange,
  onRoomCreate,
  onRoomDestroy,
  setErrorMessage,
}: {
  handleClose: () => void;
  activeRoomLink: string;
  username: string;
  onUsernameChange: (username: string) => void;
  onRoomCreate: () => void;
  onRoomDestroy: () => void;
  setErrorMessage: (message: string) => void;
}) => {
  const roomLinkInput = useRef<HTMLInputElement>(null);

  const copyRoomLink = async () => {
    try {
      await copyTextToSystemClipboard(activeRoomLink);
    } catch (error) {
      setErrorMessage(error.message);
    }
    if (roomLinkInput.current) {
      roomLinkInput.current.select();
    }
  };

  const shareRoomLink = async () => {