ronantakizawa/codereview-bench huggingface dataset:
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
    }

    this.setState({ hasError: true, info: info, error: error });

    // trigger telemetry
    dispatch(
      registerRunTimeError({ message: error.message, stack: error.stack })
    );

    dispatch(loadInconsistentObjects({ shouldReloadMetadata: true })).then(
      () => {
        if (this.props.metadata.inconsistentObjects.length > 0) {
          if (!isMetadataStatusPage()) {
            this.resetState();
            this.props.dispatch(redirectToMetadataStatus());
          }
        } else {
          console.error(error);
        }
      }
    );
  }

  render() {
    const { metadata } = this.props;
    const { hasError, type, error } = this.state as ErrorBoundaryState;

    if (hasError && metadata.ongoingRequest) {
      return (
        <div>
          <Spinner />
        </div>
      );
    }

    if (hasError) {
      return type === '404' ? (
        <PageNotFound resetCallback={this.resetState} />
      ) : (
        <RuntimeError resetCallback={this.resetState} error={error} />
      );
    }

    return this.props.children;
  }
}

export default ErrorBoundary;


Review comment (suggestion):
```suggestion
    const { hasError, type, error } = this.state;
```

Code after:

    // trigger telemetry
    dispatch(
      registerRunTimeError({ message: error.message, stack: error.stack })
    );

    dispatch(loadInconsistentObjects({ shouldReloadMetadata: true })).then(
      () => {
        if (this.props.metadata.inconsistentObjects.length > 0) {
          if (!isMetadataStatusPage()) {
            this.resetState();
            this.props.dispatch(redirectToMetadataStatus());
          }
        } else {
          console.error(error);
        }
      }
    );
  }

  resetState = () => {
    this.setState(initialState);
  };

  render() {
    const { metadata } = this.props;
    const { hasError, type, error } = this.state;

    if (hasError && metadata.ongoingRequest) {
      return <Spinner />;
    }

    if (hasError) {
      return type === '404' ? (
        <PageNotFound resetCallback={this.resetState} />
      ) : (
        <RuntimeError resetCallback={this.resetState} error={error} />
      );
    }

    return this.props.children;
  }
}

export default ErrorBoundary;
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
import React from 'react';
import {
  loadInconsistentObjects,
  redirectToMetadataStatus,
  isMetadataStatusPage,
} from '../Services/Settings/Actions';
import Spinner from '../Common/Spinner/Spinner';

import PageNotFound, { NotFoundError } from './PageNotFound';
import RuntimeError from './RuntimeError';
import { registerRunTimeError } from '../Main/Actions';

export type DefaultState = {
  inconsistentObjects: Array<object>;
  ongoingRequest: boolean;
  allowedQueries: Array<object>;
}

export interface ErrorBoundaryProps {
  metadata: DefaultState;
  dispatch: (arg: object) => Promise<object>;
}

type ErrorBoundaryState = {
  hasError: boolean;
  info: object | null;
  error: Error | null;
  type: string;
} 

class ErrorBoundary extends React.Component<ErrorBoundaryProps> {
  initialState: ErrorBoundaryState = {
    hasError: false,
    info: null,
    error: null,
    type: '500',
  };

  constructor(props: ErrorBoundaryProps) {
    super(props);

    this.state = this.initialState;
  }

  resetState = () => {
    this.setState({ ...this.initialState });

Review comment (suggestion):
Can you add this comment?
```suggestion
  dispatch: (arg: unknown) => Promise<unknown>; // TODO
```
When we have properly typed Redux code, we would need to replace `unknown` with a proper type.

Code after:
import React from 'react';
import {
  loadInconsistentObjects,
  redirectToMetadataStatus,
  isMetadataStatusPage,
} from '../Services/Settings/Actions';
import Spinner from '../Common/Spinner/Spinner';

import PageNotFound, { NotFoundError } from './PageNotFound';
import RuntimeError from './RuntimeError';
import { registerRunTimeError } from '../Main/Actions';

export interface Metadata {
  inconsistentObjects: object[];
  ongoingRequest: boolean;
  allowedQueries: object[];
}

export interface ErrorBoundaryProps {
  metadata: Metadata;
  dispatch: (arg: unknown) => Promise<unknown>; // TODO update when Redux is migrated to TS;
}

interface ErrorBoundaryState {
  hasError: boolean;
  error: Error | null;
  type: string;
}

const initialState: ErrorBoundaryState = {
  hasError: false,
  error: null,
  type: '500',
};

class ErrorBoundary extends React.Component<
  ErrorBoundaryProps,
  ErrorBoundaryState
> {
  constructor(props: ErrorBoundaryProps) {
    super(props);

    this.state = initialState;
  }

  componentDidCatch(error: Error) {
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
import React from 'react';
import {
  loadInconsistentObjects,
  redirectToMetadataStatus,
  isMetadataStatusPage,
} from '../Services/Settings/Actions';
import Spinner from '../Common/Spinner/Spinner';

import PageNotFound, { NotFoundError } from './PageNotFound';
import RuntimeError from './RuntimeError';
import { registerRunTimeError } from '../Main/Actions';

export type DefaultState = {
  inconsistentObjects: Array<object>;
  ongoingRequest: boolean;
  allowedQueries: Array<object>;
}

export interface ErrorBoundaryProps {
  metadata: DefaultState;
  dispatch: (arg: object) => Promise<object>;
}

type ErrorBoundaryState = {
  hasError: boolean;
  info: object | null;
  error: Error | null;
  type: string;
} 

class ErrorBoundary extends React.Component<ErrorBoundaryProps> {
  initialState: ErrorBoundaryState = {
    hasError: false,
    info: null,
    error: null,
    type: '500',
  };

  constructor(props: ErrorBoundaryProps) {

Review comment (suggestion):
Just for consistency. If type is simple, let's stick to `[]`. If it's complex, then use `Array`. 
```suggestion
  inconsistentObjects: object[];
```

Code after:
import React from 'react';
import {
  loadInconsistentObjects,
  redirectToMetadataStatus,
  isMetadataStatusPage,
} from '../Services/Settings/Actions';
import Spinner from '../Common/Spinner/Spinner';

import PageNotFound, { NotFoundError } from './PageNotFound';
import RuntimeError from './RuntimeError';
import { registerRunTimeError } from '../Main/Actions';

export interface Metadata {
  inconsistentObjects: object[];
  ongoingRequest: boolean;
  allowedQueries: object[];
}

export interface ErrorBoundaryProps {
  metadata: Metadata;
  dispatch: (arg: unknown) => Promise<unknown>; // TODO update when Redux is migrated to TS;
}

interface ErrorBoundaryState {
  hasError: boolean;
  error: Error | null;
  type: string;
}

const initialState: ErrorBoundaryState = {
  hasError: false,
  error: null,
  type: '500',
};

class ErrorBoundary extends React.Component<
  ErrorBoundaryProps,
  ErrorBoundaryState
> {
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
import React from 'react';
import {
  loadInconsistentObjects,
  redirectToMetadataStatus,
  isMetadataStatusPage,
} from '../Services/Settings/Actions';
import Spinner from '../Common/Spinner/Spinner';

import PageNotFound, { NotFoundError } from './PageNotFound';
import RuntimeError from './RuntimeError';
import { registerRunTimeError } from '../Main/Actions';

export type Metadata = {
  inconsistentObjects: object[];
  ongoingRequest: boolean;
  allowedQueries: object[];
};

export interface ErrorBoundaryProps {
  metadata: Metadata;
  dispatch: (arg: unknown) => Promise<unknown>; // TODO update when Redux is migrated to TS;
}

type ErrorBoundaryState = {
  hasError: boolean;
  info: object | null;
  error: Error | null;
  type: string;
};

class ErrorBoundary extends React.Component<
  ErrorBoundaryProps,
  ErrorBoundaryState
> {
  constructor(props: ErrorBoundaryProps) {
    super(props);

    this.state = this.initialState;

Review comment (suggestion):
For consistency
```suggestion
export interface Metadata {
```

Code after:
import React from 'react';
import {
  loadInconsistentObjects,
  redirectToMetadataStatus,
  isMetadataStatusPage,
} from '../Services/Settings/Actions';
import Spinner from '../Common/Spinner/Spinner';

import PageNotFound, { NotFoundError } from './PageNotFound';
import RuntimeError from './RuntimeError';
import { registerRunTimeError } from '../Main/Actions';

export interface Metadata {
  inconsistentObjects: object[];
  ongoingRequest: boolean;
  allowedQueries: object[];
}

export interface ErrorBoundaryProps {
  metadata: Metadata;
  dispatch: (arg: unknown) => Promise<unknown>; // TODO update when Redux is migrated to TS;
}

interface ErrorBoundaryState {
  hasError: boolean;
  error: Error | null;
  type: string;
}

const initialState: ErrorBoundaryState = {
  hasError: false,
  error: null,
  type: '500',
};

class ErrorBoundary extends React.Component<
  ErrorBoundaryProps,
  ErrorBoundaryState
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
      registerRunTimeError({ message: error.message, stack: error.stack })
    );

    dispatch(loadInconsistentObjects({ shouldReloadMetadata: true })).then(
      () => {
        if (this.props.metadata.inconsistentObjects.length > 0) {
          if (!isMetadataStatusPage()) {
            this.resetState();
            this.props.dispatch(redirectToMetadataStatus());
          }
        } else {
          console.error(error);
        }
      }
    );
  }

  initialState = {
    hasError: false,
    info: null,
    error: null,
    type: '500',
  };

  resetState = () => {
    this.setState({ ...this.initialState });
  };

  render() {
    const { metadata } = this.props;
    const { hasError, type, info, error } = this.state;

    if (hasError && metadata.ongoingRequest) {
      return (
        <div>
          <Spinner />
        </div>
      );
    }

    if (hasError) {
      return type === '404' ? (
        <PageNotFound resetCallback={this.resetState} />
      ) : (
        <RuntimeError
          resetCallback={this.resetState}
          error={error}
          info={info}
        />
      );
    }

Review comment (suggestion):
I know it's old code, but could you change it to the code below as there's no need to copy here:
```suggestion
    this.setState(this.initialState);
```

Code after:

    this.setState({ hasError: true, error });

    // trigger telemetry
    dispatch(
      registerRunTimeError({ message: error.message, stack: error.stack })
    );

    dispatch(loadInconsistentObjects({ shouldReloadMetadata: true })).then(
      () => {
        if (this.props.metadata.inconsistentObjects.length > 0) {
          if (!isMetadataStatusPage()) {
            this.resetState();
            this.props.dispatch(redirectToMetadataStatus());
          }
        } else {
          console.error(error);
        }
      }
    );
  }

  resetState = () => {
    this.setState(initialState);
  };

  render() {
    const { metadata } = this.props;
    const { hasError, type, error } = this.state;

    if (hasError && metadata.ongoingRequest) {
      return <Spinner />;
    }

    if (hasError) {
      return type === '404' ? (
        <PageNotFound resetCallback={this.resetState} />
      ) : (
        <RuntimeError resetCallback={this.resetState} error={error} />
      );
    }

    return this.props.children;
  }
}

export default ErrorBoundary;
Repository: hasura/graphql-engine
File: console/src/components/Common/GqlCompatibilityWarning/GqlCompatibilityWarning.tsx (TypeScript)

Code before:
import React from 'react';

import WarningSymbol from '../WarningSymbol/WarningSymbol';

const gqlPattern = /^[_A-Za-z][_0-9A-Za-z]*$/;

export interface GqlCompatibilityWarningProps {
  identifier: string;
  className?: string;
}

const GqlCompatibilityWarning: React.FC<GqlCompatibilityWarningProps> = ({ identifier, className }) => {
  const isGraphQLCompatible = gqlPattern.test(identifier);

  if (isGraphQLCompatible) {
    return null;
  }

  const gqlCompatibilityTip =
    'This identifier name does not conform to the GraphQL naming standard. ' +
    'Names in GraphQL should be limited to this ASCII subset: /[_A-Za-z][_0-9A-Za-z]*/. ' +
    'All GraphQL types depending on this identifier will not be exposed over the GraphQL API';

  return (
    <span className={className}>
      <WarningSymbol tooltipText={gqlCompatibilityTip} />
    </span>
  );
};

export default GqlCompatibilityWarning;


Review comment (suggestion):
Let's add `className = ''`. Or `className || ''` in line 25.
This way we won't end up with `undefined` as a class name 😊

Code after:
import React from 'react';

import WarningSymbol from '../WarningSymbol/WarningSymbol';

const gqlPattern = /^[_A-Za-z][_0-9A-Za-z]*$/;

export interface GqlCompatibilityWarningProps {
  identifier: string;
  className?: string;
}

const GqlCompatibilityWarning: React.FC<GqlCompatibilityWarningProps> = ({
  identifier,
  className = '',
}) => {
  const isGraphQLCompatible = gqlPattern.test(identifier);

  if (isGraphQLCompatible) {
    return null;
  }

  const gqlCompatibilityTip =
    'This identifier name does not conform to the GraphQL naming standard. ' +
    'Names in GraphQL should be limited to this ASCII subset: /[_A-Za-z][_0-9A-Za-z]*/. ' +
    'All GraphQL types depending on this identifier will not be exposed over the GraphQL API';

  return (
    <span className={className}>
      <WarningSymbol tooltipText={gqlCompatibilityTip} />
    </span>
  );
};

export default GqlCompatibilityWarning;
Repository: hasura/graphql-engine
File: console/src/components/Services/Data/Add/AddTable.js (JavaScript)

Code before:
    this.validateColumnTypes = this.validateColumnTypes.bind(this);
    this.validateColumnDefaults = this.validateColumnDefaults.bind(this);
    this.minPrimaryKeyCheck = this.minPrimaryKeyCheck.bind(this);
    this.isModified = this.isModified.bind(this);
    this.isValidType = this.isValidType.bind(this);
    this.isValidDefault = this.isValidDefault.bind(this);
  }

  componentDidMount() {
    this.props.dispatch(fetchColumnTypeInfo());
  }

  componentWillUnmount() {
    this.props.dispatch(setDefaults());
    this.props.dispatch({
      type: RESET_COLUMN_TYPE_INFO,
    });
  }

  onTableNameChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableName(e.target.value));
  };

  trimTableName = tableName => {
    const trimmedName = tableName != null ? tableName.trim() : tableName;
    const { dispatch } = this.props;
    dispatch(setTableName(trimmedName));
    return trimmedName;
  };

  onTableCommentChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableComment(e.target.value));
  };

  onRemoveColumn = i => {
    const { dispatch } = this.props;
    dispatch(removeColumn(i));
  };

  onColumnNameChange = (i, isNullableChecked, e) => {
    const { dispatch } = this.props;
    dispatch(setColName(e.target.value, i, isNullableChecked));
  };

  onColTypeChange = (i, value) => {
    const { dispatch } = this.props;
    dispatch(setColType(value, i));
  };
  onColNullableChange = (i, e) => {

Review comment (suggestion):
```suggestion
    const trimmedName = tableName ? tableName.trim() : tableName;
```

Code after:
    this.validateColumnTypes = this.validateColumnTypes.bind(this);
    this.validateColumnDefaults = this.validateColumnDefaults.bind(this);
    this.minPrimaryKeyCheck = this.minPrimaryKeyCheck.bind(this);
    this.isModified = this.isModified.bind(this);
    this.isValidType = this.isValidType.bind(this);
    this.isValidDefault = this.isValidDefault.bind(this);
  }

  componentDidMount() {
    this.props.dispatch(fetchColumnTypeInfo());
  }

  componentWillUnmount() {
    this.props.dispatch(setDefaults());
    this.props.dispatch({
      type: RESET_COLUMN_TYPE_INFO,
    });
  }

  onTableNameChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableName(e.target.value));
  };

  trimTableName = tableName => {
    const trimmedName = tableName ? tableName.trim() : tableName;
    const { dispatch } = this.props;
    if (tableName !== trimmedName) {
      dispatch(setTableName(trimmedName));
    }
    return trimmedName;
  };

  onTableCommentChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableComment(e.target.value));
  };

  onRemoveColumn = i => {
    const { dispatch } = this.props;
    dispatch(removeColumn(i));
  };

  onColumnNameChange = (i, isNullableChecked, e) => {
    const { dispatch } = this.props;
    dispatch(setColName(e.target.value, i, isNullableChecked));
  };

  onColTypeChange = (i, value) => {
    const { dispatch } = this.props;
    dispatch(setColType(value, i));
Repository: hasura/graphql-engine
File: console/src/components/Services/Data/Add/AddTable.js (JavaScript)

Code before:
    this.minPrimaryKeyCheck = this.minPrimaryKeyCheck.bind(this);
    this.isModified = this.isModified.bind(this);
    this.isValidType = this.isValidType.bind(this);
    this.isValidDefault = this.isValidDefault.bind(this);
  }

  componentDidMount() {
    this.props.dispatch(fetchColumnTypeInfo());
  }

  componentWillUnmount() {
    this.props.dispatch(setDefaults());
    this.props.dispatch({
      type: RESET_COLUMN_TYPE_INFO,
    });
  }

  onTableNameChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableName(e.target.value));
  };

  trimTableName = tableName => {
    const trimmedName = tableName != null ? tableName.trim() : tableName;
    const { dispatch } = this.props;
    dispatch(setTableName(trimmedName));
    return trimmedName;
  };

  onTableCommentChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableComment(e.target.value));
  };

  onRemoveColumn = i => {
    const { dispatch } = this.props;
    dispatch(removeColumn(i));
  };

  onColumnNameChange = (i, isNullableChecked, e) => {
    const { dispatch } = this.props;
    dispatch(setColName(e.target.value, i, isNullableChecked));
  };

  onColTypeChange = (i, value) => {
    const { dispatch } = this.props;
    dispatch(setColType(value, i));
  };
  onColNullableChange = (i, e) => {
    const { dispatch } = this.props;
    dispatch(setColNullable(e.target.checked, i));

Review comment (suggestion):
```suggestion
if (tableName !== trimmedName)  dispatch(setTableName(trimmedName));
```

Code after:
    this.minPrimaryKeyCheck = this.minPrimaryKeyCheck.bind(this);
    this.isModified = this.isModified.bind(this);
    this.isValidType = this.isValidType.bind(this);
    this.isValidDefault = this.isValidDefault.bind(this);
  }

  componentDidMount() {
    this.props.dispatch(fetchColumnTypeInfo());
  }

  componentWillUnmount() {
    this.props.dispatch(setDefaults());
    this.props.dispatch({
      type: RESET_COLUMN_TYPE_INFO,
    });
  }

  onTableNameChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableName(e.target.value));
  };

  trimTableName = tableName => {
    const trimmedName = tableName ? tableName.trim() : tableName;
    const { dispatch } = this.props;
    if (tableName !== trimmedName) {
      dispatch(setTableName(trimmedName));
    }
    return trimmedName;
  };

  onTableCommentChange = e => {
    const { dispatch } = this.props;
    dispatch(setTableComment(e.target.value));
  };

  onRemoveColumn = i => {
    const { dispatch } = this.props;
    dispatch(removeColumn(i));
  };

  onColumnNameChange = (i, isNullableChecked, e) => {
    const { dispatch } = this.props;
    dispatch(setColName(e.target.value, i, isNullableChecked));
  };

  onColTypeChange = (i, value) => {
    const { dispatch } = this.props;
    dispatch(setColType(value, i));
  };
  onColNullableChange = (i, e) => {
Repository: hasura/graphql-engine
File: console/src/components/Services/Data/Add/AddTable.js (JavaScript)

Code before:

    return false;
  }

  // return a shallow copy of a columns array, trimming those columns with
  // no non-default values from the end. Invalids in the middle retained
  trimEmptyColumns(Columns) {
    let c = Columns.slice(0);

    for (let i = c.length - 1; i >= 0; i--) {
      if (
        this.isModified(c[i].name) ||
        this.isModified(c[i].type) ||
        this.isModified(c[i].default)
      ) {
        return c;
      }
      c = c.slice(0, c.length - 1);
    }
    return c;
  }

  trimColumnNames(columns) {
    const trimmedColumns = columns.map((column, index) => {
      const trimmedColumn = column.name.trim();
      this.props.dispatch(setColName(trimmedColumn, index, column.nullable));
      return {
        ...column,
        name: trimmedColumn,
      };
    });
    return trimmedColumns;
  }

  validateEnoughColumns(cols) {
    return cols.length > 0;
  }

  validateColumnNames(cols) {
    const l = cols.length;

    for (let i = 0; i < l; i++) {
      if (!gqlPattern.test(cols[i].name)) {
        return (
          'Invalid name for column ' +
          i.toString() +
          ', ' +
          cols[i].name.toString() +
          ' is Invalid. ' +
          'Column names must be letters, numbers, and _ and start with a letter'
        );

Review comment (suggestion):
```suggestion
      if (trimmedColumn !== column.name) { 
        this.props.dispatch(setColName(trimmedColumn, index, column.nullable)); 
      }
```

Code after:
    if (x === undefined) return false;
    else if (typeof x === 'string' && /^..*$/.test(x)) return true;

    return false;
  }

  // return a shallow copy of a columns array, trimming those columns with
  // no non-default values from the end. Invalids in the middle retained
  trimEmptyColumns(Columns) {
    let c = Columns.slice(0);

    for (let i = c.length - 1; i >= 0; i--) {
      if (
        this.isModified(c[i].name) ||
        this.isModified(c[i].type) ||
        this.isModified(c[i].default)
      ) {
        return c;
      }
      c = c.slice(0, c.length - 1);
    }
    return c;
  }

  trimColumnNames(columns) {
    const trimmedColumns = columns.map((column, index) => {
      const trimmedColumn = column.name.trim();
      if (trimmedColumn !== column.name) {
        this.props.dispatch(setColName(trimmedColumn, index, column.nullable));
      }
      return {
        ...column,
        name: trimmedColumn,
      };
    });
    return trimmedColumns;
  }

  validateEnoughColumns(cols) {
    return cols.length > 0;
  }

  validateColumnNames(cols) {
    const l = cols.length;

    for (let i = 0; i < l; i++) {
      if (!gqlPattern.test(cols[i].name)) {
        return (
          'Invalid name for column ' +
          i.toString() +
          ', ' +