ronantakizawa/codereview-bench huggingface dataset:
Repository: remix-run/react-router
File: packages/router/history.ts (TypeScript)

Code before:
 */
export interface Path {
  /**
   * A URL pathname, beginning with a /.
   */
  pathname: string;

  /**
   * A URL search string, beginning with a ?.
   */
  search: string;

  /**
   * A URL fragment identifier, beginning with a #.
   */
  hash: string;
}

/**
 * An entry in a history stack. A location contains information about the
 * URL path, as well as possibly some arbitrary state and a key.
 */
export interface Location extends Path {
  /**
   * A value of arbitrary data associated with this location.
   */
  state: any;

  /**
   * A unique string associated with this location. May be used to safely store
   * and retrieve data in some other storage API, like `localStorage`.
   *
   * Note: This value is always "default" on the initial location.
   */
  key: string;
}

/**
 * A change to the current location.
 */
export interface Update {
  /**
   * The action that triggered the change.
   */
  action: Action;

  /**
   * The new location.
   */
  location: Location;


Review comment (suggestion):
```suggestion
export interface Location<State = any> extends Path {
```

Code after:
 */
export interface Path {
  /**
   * A URL pathname, beginning with a /.
   */
  pathname: string;

  /**
   * A URL search string, beginning with a ?.
   */
  search: string;

  /**
   * A URL fragment identifier, beginning with a #.
   */
  hash: string;
}

// TODO: (v7) Change the Location generic default from `any` to `unknown` and
// remove Remix `useLocation` wrapper.

/**
 * An entry in a history stack. A location contains information about the
 * URL path, as well as possibly some arbitrary state and a key.
 */
export interface Location<S = any> extends Path {
  /**
   * A value of arbitrary data associated with this location.
   */
  state: S;

  /**
   * A unique string associated with this location. May be used to safely store
   * and retrieve data in some other storage API, like `localStorage`.
   *
   * Note: This value is always "default" on the initial location.
   */
  key: string;
}

/**
 * A change to the current location.
 */
export interface Update {
  /**
   * The action that triggered the change.
   */
  action: Action;

  /**
   * The new location.
Repository: remix-run/react-router
File: packages/router/utils.ts (TypeScript)

Code before:
      formMethod: FormMethod | V7_FormMethod;
      formAction: string;
      formEncType: FormEncType;
      formData: undefined;
      json: undefined;
      text: string;
    };

/**
 * @private
 * Arguments passed to route loader/action functions.  Same for now but we keep
 * this as a private implementation detail in case they diverge in the future.
 */
interface DataFunctionArgs {
  request: Request;
  params: Params;
  context?: any;
}

/**
 * Arguments passed to loader functions
 */
export interface LoaderFunctionArgs extends DataFunctionArgs {}

/**
 * Arguments passed to action functions
 */
export interface ActionFunctionArgs extends DataFunctionArgs {}

/**
 * Loaders and actions can return anything except `undefined` (`null` is a
 * valid return value if there is no data to return).  Responses are preferred
 * and will ease any future migration to Remix
 */
type DataFunctionValue = Response | NonNullable<unknown> | null;

/**
 * Route loader function signature
 */
export interface LoaderFunction {
  (args: LoaderFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Route action function signature
 */
export interface ActionFunction {
  (args: ActionFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**

Review comment (suggestion):
```suggestion
export interface LoaderFunctionArgs<Context = any> extends DataFunctionArgs<Context> {}
```

Code after:
      formMethod: FormMethod | V7_FormMethod;
      formAction: string;
      formEncType: FormEncType;
      formData: undefined;
      json: undefined;
      text: string;
    };

/**
 * @private
 * Arguments passed to route loader/action functions.  Same for now but we keep
 * this as a private implementation detail in case they diverge in the future.
 */
interface DataFunctionArgs<Context> {
  request: Request;
  params: Params;
  context?: Context;
}

// TODO: (v7) Change the defaults from any to unknown in and remove Remix wrappers:
//   ActionFunction, ActionFunctionArgs, LoaderFunction, LoaderFunctionArgs

/**
 * Arguments passed to loader functions
 */
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}

/**
 * Arguments passed to action functions
 */
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}

/**
 * Loaders and actions can return anything except `undefined` (`null` is a
 * valid return value if there is no data to return).  Responses are preferred
 * and will ease any future migration to Remix
 */
type DataFunctionValue = Response | NonNullable<unknown> | null;

/**
 * Route loader function signature
 */
export interface LoaderFunction<C = any> {
  (args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Route action function signature
 */
export interface ActionFunction<C = any> {
  (args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
Repository: remix-run/react-router
File: packages/router/utils.ts (TypeScript)

Code before:
      text: string;
    };

/**
 * @private
 * Arguments passed to route loader/action functions.  Same for now but we keep
 * this as a private implementation detail in case they diverge in the future.
 */
interface DataFunctionArgs {
  request: Request;
  params: Params;
  context?: any;
}

/**
 * Arguments passed to loader functions
 */
export interface LoaderFunctionArgs extends DataFunctionArgs {}

/**
 * Arguments passed to action functions
 */
export interface ActionFunctionArgs extends DataFunctionArgs {}

/**
 * Loaders and actions can return anything except `undefined` (`null` is a
 * valid return value if there is no data to return).  Responses are preferred
 * and will ease any future migration to Remix
 */
type DataFunctionValue = Response | NonNullable<unknown> | null;

/**
 * Route loader function signature
 */
export interface LoaderFunction {
  (args: LoaderFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Route action function signature
 */
export interface ActionFunction {
  (args: ActionFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Arguments passed to shouldRevalidate function
 */
export interface ShouldRevalidateFunctionArgs {
  currentUrl: URL;
  currentParams: AgnosticDataRouteMatch["params"];

Review comment (suggestion):
```suggestion
export interface ActionFunctionArgs<Context = any> extends DataFunctionArgs<Context> {}
```

Code after:
      text: string;
    };

/**
 * @private
 * Arguments passed to route loader/action functions.  Same for now but we keep
 * this as a private implementation detail in case they diverge in the future.
 */
interface DataFunctionArgs<Context> {
  request: Request;
  params: Params;
  context?: Context;
}

// TODO: (v7) Change the defaults from any to unknown in and remove Remix wrappers:
//   ActionFunction, ActionFunctionArgs, LoaderFunction, LoaderFunctionArgs

/**
 * Arguments passed to loader functions
 */
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}

/**
 * Arguments passed to action functions
 */
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}

/**
 * Loaders and actions can return anything except `undefined` (`null` is a
 * valid return value if there is no data to return).  Responses are preferred
 * and will ease any future migration to Remix
 */
type DataFunctionValue = Response | NonNullable<unknown> | null;

/**
 * Route loader function signature
 */
export interface LoaderFunction<C = any> {
  (args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Route action function signature
 */
export interface ActionFunction<C = any> {
  (args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Arguments passed to shouldRevalidate function
 */
Repository: remix-run/react-router
File: packages/router/utils.ts (TypeScript)

Code before:

/**
 * Arguments passed to loader functions
 */
export interface LoaderFunctionArgs extends DataFunctionArgs {}

/**
 * Arguments passed to action functions
 */
export interface ActionFunctionArgs extends DataFunctionArgs {}

/**
 * Loaders and actions can return anything except `undefined` (`null` is a
 * valid return value if there is no data to return).  Responses are preferred
 * and will ease any future migration to Remix
 */
type DataFunctionValue = Response | NonNullable<unknown> | null;

/**
 * Route loader function signature
 */
export interface LoaderFunction {
  (args: LoaderFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Route action function signature
 */
export interface ActionFunction {
  (args: ActionFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Arguments passed to shouldRevalidate function
 */
export interface ShouldRevalidateFunctionArgs {
  currentUrl: URL;
  currentParams: AgnosticDataRouteMatch["params"];
  nextUrl: URL;
  nextParams: AgnosticDataRouteMatch["params"];
  formMethod?: Submission["formMethod"];
  formAction?: Submission["formAction"];
  formEncType?: Submission["formEncType"];
  text?: Submission["text"];
  formData?: Submission["formData"];
  json?: Submission["json"];
  actionResult?: any;
  defaultShouldRevalidate: boolean;
}

/**

Review comment (suggestion):
```suggestion
export interface LoaderFunction<Context = any> {
  (args: LoaderFunctionArgs<Context>): Promise<DataFunctionValue> | DataFunctionValue;
```

Code after:

// TODO: (v7) Change the defaults from any to unknown in and remove Remix wrappers:
//   ActionFunction, ActionFunctionArgs, LoaderFunction, LoaderFunctionArgs

/**
 * Arguments passed to loader functions
 */
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}

/**
 * Arguments passed to action functions
 */
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}

/**
 * Loaders and actions can return anything except `undefined` (`null` is a
 * valid return value if there is no data to return).  Responses are preferred
 * and will ease any future migration to Remix
 */
type DataFunctionValue = Response | NonNullable<unknown> | null;

/**
 * Route loader function signature
 */
export interface LoaderFunction<C = any> {
  (args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Route action function signature
 */
export interface ActionFunction<C = any> {
  (args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
 * Arguments passed to shouldRevalidate function
 */
export interface ShouldRevalidateFunctionArgs {
  currentUrl: URL;
  currentParams: AgnosticDataRouteMatch["params"];
  nextUrl: URL;
  nextParams: AgnosticDataRouteMatch["params"];
  formMethod?: Submission["formMethod"];
  formAction?: Submission["formAction"];
  formEncType?: Submission["formEncType"];
  text?: Submission["text"];
  formData?: Submission["formData"];
  json?: Submission["json"];
  actionResult?: any;
  defaultShouldRevalidate: boolean;
Repository: remix-run/react-router
File: packages/router/utils.ts (TypeScript)

Code before:
  }

  let branches = flattenRoutes(routes);
  rankRouteBranches(branches);

  let matches = null;
  for (let i = 0; matches == null && i < branches.length; ++i) {
    matches = matchRouteBranch<string, RouteObjectType>(
      branches[i],
      // Incoming pathnames are generally encoded from either window.location
      // or from router.navigate, but we want to match against the unencoded
      // paths in the route definitions.  Memory router locations won't be
      // encoded here but there also shouldn't be anything to decode so this
      // should be a safe operation.  This avoids needing matchRoutes to be
      // history-aware.
      safelyDecodeURI(pathname)
    );
  }

  return matches;
}

interface RouteMeta<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
  relativePath: string;
  caseSensitive: boolean;
  childrenIndex: number;
  route: RouteObjectType;
}

interface RouteBranch<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
  path: string;
  score: number;
  routesMeta: RouteMeta<RouteObjectType>[];
}

function flattenRoutes<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
>(
  routes: RouteObjectType[],
  branches: RouteBranch<RouteObjectType>[] = [],
  parentsMeta: RouteMeta<RouteObjectType>[] = [],
  parentPath = ""
): RouteBranch<RouteObjectType>[] {
  let flattenRoute = (
    route: RouteObjectType,
    index: number,
    relativePath?: string

Review comment (suggestion):
```suggestion
export interface UIMatch<Data = unknown, Handle = unknown> {
```

Code after:

  if (pathname == null) {
    return null;
  }

  let branches = flattenRoutes(routes);
  rankRouteBranches(branches);

  let matches = null;
  for (let i = 0; matches == null && i < branches.length; ++i) {
    matches = matchRouteBranch<string, RouteObjectType>(
      branches[i],
      // Incoming pathnames are generally encoded from either window.location
      // or from router.navigate, but we want to match against the unencoded
      // paths in the route definitions.  Memory router locations won't be
      // encoded here but there also shouldn't be anything to decode so this
      // should be a safe operation.  This avoids needing matchRoutes to be
      // history-aware.
      safelyDecodeURI(pathname)
    );
  }

  return matches;
}

export interface UIMatch<D = unknown, H = unknown> {
  id: string;
  pathname: string;
  params: AgnosticRouteMatch["params"];
  data: D;
  handle: H;
}

export function convertRouteMatchToUiMatch(
  match: AgnosticDataRouteMatch,
  loaderData: RouteData
): UIMatch {
  let { route, pathname, params } = match;
  return {
    id: route.id,
    pathname,
    params,
    data: loaderData[route.id],
    handle: route.handle,
  };
}

interface RouteMeta<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
  relativePath: string;
Repository: remix-run/react-router
File: packages/router/utils.ts (TypeScript)

Code before:
  let matches = null;
  for (let i = 0; matches == null && i < branches.length; ++i) {
    matches = matchRouteBranch<string, RouteObjectType>(
      branches[i],
      // Incoming pathnames are generally encoded from either window.location
      // or from router.navigate, but we want to match against the unencoded
      // paths in the route definitions.  Memory router locations won't be
      // encoded here but there also shouldn't be anything to decode so this
      // should be a safe operation.  This avoids needing matchRoutes to be
      // history-aware.
      safelyDecodeURI(pathname)
    );
  }

  return matches;
}

interface RouteMeta<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
  relativePath: string;
  caseSensitive: boolean;
  childrenIndex: number;
  route: RouteObjectType;
}

interface RouteBranch<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
  path: string;
  score: number;
  routesMeta: RouteMeta<RouteObjectType>[];
}

function flattenRoutes<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
>(
  routes: RouteObjectType[],
  branches: RouteBranch<RouteObjectType>[] = [],
  parentsMeta: RouteMeta<RouteObjectType>[] = [],
  parentPath = ""
): RouteBranch<RouteObjectType>[] {
  let flattenRoute = (
    route: RouteObjectType,
    index: number,
    relativePath?: string
  ) => {
    let meta: RouteMeta<RouteObjectType> = {
      relativePath:
        relativePath === undefined ? route.path || "" : relativePath,
      caseSensitive: route.caseSensitive === true,

Review comment (suggestion):
```suggestion
  data: Data;
  handle: Handle;
```

Code after:
  let branches = flattenRoutes(routes);
  rankRouteBranches(branches);

  let matches = null;
  for (let i = 0; matches == null && i < branches.length; ++i) {
    matches = matchRouteBranch<string, RouteObjectType>(
      branches[i],
      // Incoming pathnames are generally encoded from either window.location
      // or from router.navigate, but we want to match against the unencoded
      // paths in the route definitions.  Memory router locations won't be
      // encoded here but there also shouldn't be anything to decode so this
      // should be a safe operation.  This avoids needing matchRoutes to be
      // history-aware.
      safelyDecodeURI(pathname)
    );
  }

  return matches;
}

export interface UIMatch<D = unknown, H = unknown> {
  id: string;
  pathname: string;
  params: AgnosticRouteMatch["params"];
  data: D;
  handle: H;
}

export function convertRouteMatchToUiMatch(
  match: AgnosticDataRouteMatch,
  loaderData: RouteData
): UIMatch {
  let { route, pathname, params } = match;
  return {
    id: route.id,
    pathname,
    params,
    data: loaderData[route.id],
    handle: route.handle,
  };
}

interface RouteMeta<
  RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
  relativePath: string;
  caseSensitive: boolean;
  childrenIndex: number;
  route: RouteObjectType;
}
Repository: remix-run/react-router
File: packages/router/utils.ts (TypeScript)

Code before:
    ...responseInit,
    headers,
  });
};

/**
 * A redirect response that will force a document reload to the new location.
 * Sets the status code and the `Location` header.
 * Defaults to "302 Found".
 */
export const redirectDocument: RedirectFunction = (url, init) => {
  let response = redirect(url, init);
  response.headers.set("X-Remix-Reload-Document", "true");
  return response;
};

/**
 * @private
 * Utility class we use to hold auto-unwrapped 4xx/5xx Response bodies
 */
export class ErrorResponseImpl {
  status: number;
  statusText: string;
  data: any;
  private error?: Error;
  private internal: boolean;

  constructor(
    status: number,
    statusText: string | undefined,
    data: any,
    internal = false
  ) {
    this.status = status;
    this.statusText = statusText || "";
    this.internal = internal;
    if (data instanceof Error) {
      this.data = data.toString();
      this.error = data;
    } else {
      this.data = data;
    }
  }
}

// We don't want the class exported since usage of it at runtime is an
// implementation detail, but we do want to export the shape so folks can
// build their own abstractions around instances via isRouteErrorResponse()
export type ErrorResponse = InstanceType<typeof ErrorResponseImpl>;

/**

Review comment (refactor):
```suggestion
/**
 * @private
 * Utility class we use to hold auto-unwrapped 4xx/5xx Response bodies
 *
 * We don't export the class for public use since it's an implementation
 * detail, but we export the interface above so folks can build their own 
 * abstractions around instances via isRouteErrorResponse() 
 */
```



Code after:
    ...responseInit,
    headers,
  });
};

/**
 * A redirect response that will force a document reload to the new location.
 * Sets the status code and the `Location` header.
 * Defaults to "302 Found".
 */
export const redirectDocument: RedirectFunction = (url, init) => {
  let response = redirect(url, init);
  response.headers.set("X-Remix-Reload-Document", "true");
  return response;
};

export type ErrorResponse = {
  status: number;
  statusText: string;
  data: any;
};

/**
 * @private
 * Utility class we use to hold auto-unwrapped 4xx/5xx Response bodies
 *
 * We don't export the class for public use since it's an implementation
 * detail, but we export the interface above so folks can build their own
 * abstractions around instances via isRouteErrorResponse()
 */
export class ErrorResponseImpl implements ErrorResponse {
  status: number;
  statusText: string;
  data: any;
  private error?: Error;
  private internal: boolean;

  constructor(
    status: number,
    statusText: string | undefined,
    data: any,
    internal = false
  ) {
    this.status = status;
    this.statusText = statusText || "";
    this.internal = internal;
    if (data instanceof Error) {
      this.data = data.toString();
      this.error = data;
    } else {
      this.data = data;
Repository: remix-run/react-router
File: packages/router/__tests__/path-resolution-test.ts (TypeScript)

Code before:
        true
      );
    });

    it("from a dynamic param route", () => {
      assertRoutingToSelf(
        [
          {
            id: "activeRoute",
            path: ":param",
          },
        ],
        "/foo/bar",
        false
      );
    });

    it("from a splat route", () => {
      assertRoutingToSelf(
        [
          {
            id: "activeRoute",
            path: "*",
          },
        ],
        "/foo",
        false
      );
    });
    /* eslint-enable jest/expect-expect */
  });

  describe("routing to parent", () => {
    function assertRoutingToParent(fooChildren) {
      let router = createRouter({
        routes: [
          {
            path: "/",
            children: [
              {
                path: "foo",
                children: fooChildren,
              },
            ],
          },
        ],
        history: createMemoryHistory({
          initialEntries: ["/foo/bar?a=1#hash"],
        }),
      }).initialize();


Review comment (bug):
Similar to #10983, we had a test here asserting the previously buggy behavior.  A link to "." from a splat should stay on the current location, not route up to the parent

Code after:
        true
      );
    });

    it("from a dynamic param route", () => {
      assertRoutingToSelf(
        [
          {
            id: "activeRoute",
            path: ":param",
          },
        ],
        "/foo/bar",
        false
      );
    });

    it("from a splat route", () => {
      assertRoutingToSelf(
        [
          {
            id: "activeRoute",
            path: "*",
          },
        ],
        "/foo/bar",
        false
      );
    });
    /* eslint-enable jest/expect-expect */
  });

  describe("routing to parent", () => {
    function assertRoutingToParent(fooChildren) {
      let router = createRouter({
        routes: [
          {
            path: "/",
            children: [
              {
                path: "foo",
                children: fooChildren,
              },
            ],
          },
        ],
        history: createMemoryHistory({
          initialEntries: ["/foo/bar?a=1#hash"],
        }),
      }).initialize();