ronantakizawa/codereview-bench huggingface dataset:
Repository: hasura/graphql-engine
File: console/src/telemetry/index.ts (TypeScript)

Code before:
};

let client = createClient();
if (client) {
  const onClose = () => {
    client = createClient();
    if (client) {
      client.onclose = onClose;
    }
  };
  client.onclose = onClose;
}

const isTelemetryConnectionReady = () => {
  return !!(client && client.readyState === client.OPEN);
};

const sendEvent = (payload: any) => {
  if (client && isTelemetryConnectionReady()) {
    client.send(
      JSON.stringify({ data: payload, topic: globals.telemetryTopic })
    );
  }
};

export const trackReduxAction = (action: ReduxAction, getState: any) => {
  const actionType = action.type;
  // filter events
  if (!filterEventsBlockList.includes(actionType)) {
    const serverVersion = getState().main.serverVersion;
    const url = sanitiseUrl(window.location.pathname);

    const reqBody = {
      server_version: serverVersion,
      event_type: actionType,
      url,
      console_mode: globals.consoleMode,
      cli_uuid: globals.cliUUID,
      server_uuid: getState().telemetry.hasura_uuid,
      data: null,
    };

    const isLocationType = actionType === REDUX_LOCATION_CHANGE_ACTION_TYPE;
    if (isLocationType) {
      // capture page views
      const payload = action.payload;
      reqBody.url = sanitiseUrl(payload.pathname);
    }

    const isErrorType = actionType === RUN_TIME_ERROR;
    if (isErrorType) {

Review comment (suggestion):
Maybe we can say at least that this `getState` is a function that returns an object?
`getState: () => Record<string, any>`

Code after:
      client.onclose = onClose;
    }
  };
  client.onclose = onClose;
}

const isTelemetryConnectionReady = () => {
  return !!(client && client.readyState === client.OPEN);
};

const sendEvent = (payload: any) => {
  if (client && isTelemetryConnectionReady()) {
    client.send(
      JSON.stringify({ data: payload, topic: globals.telemetryTopic })
    );
  }
};

export const trackReduxAction = (
  action: ReduxAction,
  getState: GetReduxState
) => {
  const actionType = action.type;
  // filter events
  if (!filterEventsBlockList.includes(actionType)) {
    const serverVersion = getState().main.serverVersion;
    const url = sanitiseUrl(window.location.pathname);

    const reqBody = {
      server_version: serverVersion,
      event_type: actionType,
      url,
      console_mode: globals.consoleMode,
      cli_uuid: globals.cliUUID,
      server_uuid: getState().telemetry.hasura_uuid,
      data: null,
    };

    const isLocationType = actionType === REDUX_LOCATION_CHANGE_ACTION_TYPE;
    if (isLocationType) {
      // capture page views
      const payload = action.payload;
      reqBody.url = sanitiseUrl(payload.pathname);
    }

    const isErrorType = actionType === RUN_TIME_ERROR;
    if (isErrorType) {
      reqBody.data = action.data;
    }

    // Send the data
Repository: hasura/graphql-engine
File: cli/cli.go (Go)

Code before:
}

// setupSpinner creates a default spinner if the context does not already have
// one.
func (ec *ExecutionContext) setupSpinner() {
	if ec.Spinner == nil {
		spnr := spinner.New(spinner.CharSets[7], 100*time.Millisecond)
		spnr.Writer = os.Stderr
		ec.Spinner = spnr
	}
}

// Spin stops any existing spinner and starts a new one with the given message.
func (ec *ExecutionContext) Spin(message string) {
	if ec.IsTerminal {
		ec.Spinner.Stop()
		ec.Spinner.Prefix = message
		ec.Spinner.Start()
	} else {
		ec.Logger.Println(message)
	}
}

// loadEnvfile loads .env file
func (ec *ExecutionContext) loadEnvfile() error {
	ec.Envfile += ".env"
	envfile := filepath.Join(ec.ExecutionDirectory, ec.Envfile)
	err := gotenv.Load(envfile)
	if err != nil {
		// return error if user provided envfile name
		if ec.Envfile != ".env" {
			return err
		} else {
			if !os.IsNotExist(err) {
				ec.Logger.Warn(err)
			}
		}
	}
	return nil
}

// setupLogger creates a default logger if context does not have one set.
func (ec *ExecutionContext) setupLogger() {
	if ec.Logger == nil {
		logger := logrus.New()
		ec.Logger = logger
	}

	if ec.LogLevel != "" {
		level, err := logrus.ParseLevel(ec.LogLevel)
		if err != nil {

Review comment (suggestion):
User should be able to give the complete file path instead of just the name and us adding the extension. And `.env` should work by default.

Code after:
			HandlerWebhookBaseURL: v.GetString("actions.handler_webhook_baseurl"),
			Codegen: &types.CodegenExecutionConfig{
				Framework: v.GetString("actions.codegen.framework"),
				OutputDir: v.GetString("actions.codegen.output_dir"),
				URI:       v.GetString("actions.codegen.uri"),
			},
		},
	}
	if !ec.Config.Version.IsValid() {
		return ErrInvalidConfigVersion
	}
	return ec.Config.ServerConfig.ParseEndpoint()
}

// setupSpinner creates a default spinner if the context does not already have
// one.
func (ec *ExecutionContext) setupSpinner() {
	if ec.Spinner == nil {
		spnr := spinner.New(spinner.CharSets[7], 100*time.Millisecond)
		spnr.Writer = os.Stderr
		ec.Spinner = spnr
	}
}

// Spin stops any existing spinner and starts a new one with the given message.
func (ec *ExecutionContext) Spin(message string) {
	if ec.IsTerminal {
		ec.Spinner.Stop()
		ec.Spinner.Prefix = message
		ec.Spinner.Start()
	} else {
		ec.Logger.Println(message)
	}
}

// loadEnvfile loads .env file
func (ec *ExecutionContext) loadEnvfile() error {
	envfile := filepath.Join(ec.ExecutionDirectory, ec.Envfile)
	err := gotenv.Load(envfile)
	if err != nil {
		// return error if user provided envfile name
		if ec.Envfile != ".env" {
			return err
		}
		if !os.IsNotExist(err) {
			ec.Logger.Warn(err)
		}
	}
	if err == nil {
		ec.Logger.Debug("ENV vars read from: ", envfile)
	}
Repository: hasura/graphql-engine
File: console/src/declaration.d.ts (TypeScript)

Code before:
import { CSSProp } from 'styled-components';

declare module '*.scss' {
  const content: { [className: string]: string };
  export default content;
}

declare namespace SvgPanZoom {
  export interface Instance {}
}
declare module 'react' {
  interface HTMLAttributes<T> extends DOMAttributes<T> {
    css?: CSSProp;
  }
}


Review comment (suggestion):
It is fix this type error.

https://github.com/hasura/graphql-engine/pull/4504/commits/21a64fdf7adf07f6f2522a27f1411de7a645e92b#diff-2045ac77f10a7a2f5b039fa2460c841eR56

Code after:
declare module '*.scss' {
  const content: { [className: string]: string };
  export default content;
}

declare namespace SvgPanZoom {
  export interface Instance {}
}

declare namespace React {
  interface HTMLAttributes<T> extends DOMAttributes<T> {
    css?: import('styled-components').CSSProp;
  }
}
Repository: hasura/graphql-engine
File: console/src/components/UIKit/atoms/Spinner/index.tsx (TypeScript)

Code before:

const spinnerChildStyles = css`
  width: 100%;
  height: 100%;
  position: absolute;
  left: 0;
  top: 0;

  &:before {
    content: '';
    display: block;
    margin: 0 auto;
    width: 15%;
    height: 15%;
    border-radius: 100%;
    animation: ${circleBounceDelay} 1.2s infinite ease-in-out both;

    background-color: #333;
  }
`;

export type SpinnerProps = {
  size: string
};

export const Spinner: React.FC<SpinnerProps> = props => {
  const { size } = props;

  const spinnerWidth = size === 'small' ? smallSpinnerSize : largeSpinnerSize;

  const spinnerHeight = size === 'small' ? smallSpinnerSize : largeSpinnerSize;

  return (
    <StyledSpinner {...props} height={spinnerHeight} width={spinnerWidth}>
      {Array.from(new Array(12), (_, i) => i).map(i => (
        <div
          key={i}
          css={css`
              ${spinnerChildStyles}
              transform: rotate(${30 * i}deg);

              &:before {
                animation-delay: ${-1.1 + i / 10}s;
              }
            `}
        />
      ))}
    </StyledSpinner>
  );
};


Review comment (suggestion):
We also need to say that all StyledSpinner props are allowed to be passed. So you can export StyledSpinner's props from `Spinner.ts` and do:
```ts
type SpinnerProps = { ... } & StyledSpinnerProps
```

Code after:

const spinnerChildStyles = css`
  width: 100%;
  height: 100%;
  position: absolute;
  left: 0;
  top: 0;

  &:before {
    content: '';
    display: block;
    margin: 0 auto;
    width: 15%;
    height: 15%;
    border-radius: 100%;
    animation: ${circleBounceDelay} 1.2s infinite ease-in-out both;

    background-color: #333;
  }
`;

export interface SpinnerProps extends StyledSpinnerProps {
  size: string;
}

export const Spinner: React.FC<SpinnerProps> = props => {
  const { size } = props;

  const spinnerWidth = size === 'small' ? smallSpinnerSize : largeSpinnerSize;

  const spinnerHeight = size === 'small' ? smallSpinnerSize : largeSpinnerSize;

  return (
    <StyledSpinner {...props} height={spinnerHeight} width={spinnerWidth}>
      {Array.from(new Array(12), (_, i) => i).map(i => (
        <div
          key={i}
          css={css`
              ${spinnerChildStyles}
              transform: rotate(${30 * i}deg);

              &:before {
                animation-delay: ${-1.1 + i / 10}s;
              }
            `}
        />
      ))}
    </StyledSpinner>
  );
};
Repository: hasura/graphql-engine
File: console/src/components/UIKit/atoms/SwitchButton/index.tsx (TypeScript)

Code before:
import React, { useState } from 'react';

import { StyledSwitchButton, StyledSlider } from './SwitchButton';

export const SwitchButton: React.FC = props => {
  const [isChecked, toggleCheckbox] = useState(false);
  const { children } = props;

  return (
    <StyledSwitchButton {...props}>
      <label>
        <input type="checkbox" onClick={() => toggleCheckbox(!isChecked)} />
        <StyledSlider checked={isChecked} />
        {children}
      </label>
    </StyledSwitchButton>
  );
};


Review comment (suggestion):
`SwicthButton` is accepting props: https://github.com/hasura/graphql-engine/blob/76e1e0286c441ec84001f3871750c22a40c1d504/console/src/components/UIKit/atoms/SwitchButton/index.tsx#L10 so we need to do the same thing as in the case of `Spinner` component.

Code after:
import React, { useState } from 'react';

import {
  StyledSwitchButton,
  StyledSlider,
  StyledSwitchButtonProps,
} from './SwitchButton';

interface SwitchButtonProps extends StyledSwitchButtonProps {}

export const SwitchButton: React.FC<SwitchButtonProps> = props => {
  const [isChecked, toggleCheckbox] = useState(false);
  const { children } = props;

  return (
    <StyledSwitchButton {...props}>
      <label>
        <input type="checkbox" onClick={() => toggleCheckbox(!isChecked)} />
        <StyledSlider checked={isChecked} />
        {children}
      </label>
    </StyledSwitchButton>
  );
};
Repository: hasura/graphql-engine
File: console/src/components/Common/utils/sqlUtils.ts (TypeScript)

Code before:

export const getCreatePkSql = (options: SqlUtilsOptions) => {
  const { schemaName, tableName, selectedPkColumns, constraintName } = options;

  // if no primary key columns provided, return empty query
  if (!selectedPkColumns || selectedPkColumns.length === 0) {
    return '';
  }

  return `alter table "${schemaName}"."${tableName}"
    add constraint "${constraintName}"
    primary key ( ${selectedPkColumns
      .map((pkc: string) => `"${pkc}"`)
      .join(', ')} );`;
};

export const getDropPkSql = (options: SqlUtilsOptions) => {
  const { schemaName, tableName, constraintName } = options;

  return `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}";`;
};

export const terminateSql = (sql: string) => {
  const sqlSanitised = sql.trim();

  return sqlSanitised[sqlSanitised.length - 1] !== ';' || sqlSanitised;
};


Review comment (suggestion):
The logic before was slightly different:
if there's no `;` at the end add `;`, otherwise, return `sqlSanitised`.
```js
 return sqlSanitised[sqlSanitised.length - 1] !== ';'	
    ? sqlSanitised + ';'	
    : sqlSanitised;
```


Code after:
  schemaName,
  tableName,
  selectedPkColumns,
  constraintName,
}: SqlUtilsOptions) => {
  // if no primary key columns provided, return empty query
  if (!selectedPkColumns || selectedPkColumns.length === 0) {
    return '';
  }

  return `alter table "${schemaName}"."${tableName}"
    add constraint "${constraintName}"
    primary key ( ${selectedPkColumns.map(pkc => `"${pkc}"`).join(', ')} );`;
};

export const getDropPkSql = ({
  schemaName,
  tableName,
  constraintName,
}: SqlUtilsOptions) => {
  return `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}";`;
};

export const terminateSql = (sql: string) => {
  const sqlTerminated = sql.trim();

  return sqlTerminated[sqlTerminated.length - 1] !== ';'
    ? `${sqlTerminated};`
    : sqlTerminated;
};
Repository: hasura/graphql-engine
File: console/src/components/Common/utils/sqlUtils.ts (TypeScript)

Code before:
  schemaName: string;
  constraintName: string;
  check?: string;
  selectedPkColumns?: string[];
}

// all function could benefit from above interface for uniform function systax
// requires editing other files to update call references, so will update in next iteration
export const sqlEscapeText = (text: string) => {
  let escapedText = text;

  if (escapedText) {
    escapedText = escapedText.replace(/'/g, "\\'");
  }

  return `E'${escapedText}'`;
};

// detect DDL statements in SQL
export const checkSchemaModification = (_sql: string) => {
  let isSchemaModification = false;

  const sqlStatements = _sql
    .toLowerCase()
    .split(';')
    .map((s: string) => s.trim());

  sqlStatements.forEach((statement: string) => {
    if (
      statement.startsWith('create ') ||
      statement.startsWith('alter ') ||
      statement.startsWith('drop ')
    ) {
      isSchemaModification = true;
    }
  });

  return isSchemaModification;
};

export const getCheckConstraintBoolExp = (check: string) => {
  if (check) {
    return check.substring(7, check.length - 1);
  }

  return check;
};

/* queries */

export const getCreateCheckConstraintSql = (

Review comment (suggestion):
```suggestion
    .map(s=> s.trim());
```
TypeScript infers type for `s`.

Code after:
  schemaName: string;
  constraintName: string;
  check?: string;
  selectedPkColumns?: string[];
}

export const sqlEscapeText = (text: string) => {
  let escapedText = text;

  if (escapedText) {
    escapedText = escapedText.replace(/'/g, "\\'");
  }

  return `E'${escapedText}'`;
};

// detect DDL statements in SQL
export const checkSchemaModification = (_sql: string) => {
  let isSchemaModification = false;

  const sqlStatements = _sql
    .toLowerCase()
    .split(';')
    .map(s => s.trim());

  sqlStatements.forEach((statement: string) => {
    if (
      statement.startsWith('create ') ||
      statement.startsWith('alter ') ||
      statement.startsWith('drop ')
    ) {
      isSchemaModification = true;
    }
  });

  return isSchemaModification;
};

export const getCheckConstraintBoolExp = (check: string) => {
  if (check) {
    return check.substring(7, check.length - 1);
  }

  return check;
};

/* queries */

export const getCreateCheckConstraintSql = (
  tableName: string,
  schemaName: string,
Repository: hasura/graphql-engine
File: console/src/components/Common/utils/sqlUtils.ts (TypeScript)

Code before:
};

export const getDropConstraintSql = (
  tableName: string,
  schemaName: string,
  constraintName: string
) => {
  return `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}"`;
};

export const getCreatePkSql = (options: SqlUtilsOptions) => {
  const { schemaName, tableName, selectedPkColumns, constraintName } = options;

  // if no primary key columns provided, return empty query
  if (!selectedPkColumns || selectedPkColumns.length === 0) {
    return '';
  }

  return `alter table "${schemaName}"."${tableName}"
    add constraint "${constraintName}"
    primary key ( ${selectedPkColumns
      .map((pkc: string) => `"${pkc}"`)
      .join(', ')} );`;
};

export const getDropPkSql = (options: SqlUtilsOptions) => {
  const { schemaName, tableName, constraintName } = options;

  return `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}";`;
};

export const terminateSql = (sql: string) => {
  const sqlTerminated = sql.trim();

  return sqlTerminated[sqlTerminated.length - 1] !== ';'
    ? `${sqlTerminated};`
    : sqlTerminated;
};


Review comment (suggestion):
Ditto, let's destruct it here: 

```suggestion
export const getDropPkSql = ({ schemaName, tableName, constraintName }: SqlUtilsOptions) => {
```

Code after:
export const getDropConstraintSql = (
  tableName: string,
  schemaName: string,
  constraintName: string
) => {
  return `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}"`;
};

export const getCreatePkSql = ({
  schemaName,
  tableName,
  selectedPkColumns,
  constraintName,
}: SqlUtilsOptions) => {
  // if no primary key columns provided, return empty query
  if (!selectedPkColumns || selectedPkColumns.length === 0) {
    return '';
  }

  return `alter table "${schemaName}"."${tableName}"
    add constraint "${constraintName}"
    primary key ( ${selectedPkColumns.map(pkc => `"${pkc}"`).join(', ')} );`;
};

export const getDropPkSql = ({
  schemaName,
  tableName,
  constraintName,
}: SqlUtilsOptions) => {
  return `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}";`;
};

export const terminateSql = (sql: string) => {
  const sqlTerminated = sql.trim();

  return sqlTerminated[sqlTerminated.length - 1] !== ';'
    ? `${sqlTerminated};`
    : sqlTerminated;
};
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
} 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 });
  };

  componentDidCatch(error: Error, info: object) {
    const { dispatch } = this.props;

    // for invalid path segment errors
    if (error instanceof NotFoundError) {
      this.setState({
        type: '404',
      });

Review comment (suggestion):
```suggestion
class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState > {
```

Code after:
} 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) {
    const { dispatch } = this.props;

    // for invalid path segment errors
    if (error instanceof NotFoundError) {
      this.setState({
        type: '404',
      });
    }

    this.setState({ hasError: true, error });
Repository: hasura/graphql-engine
File: console/src/components/Error/ErrorBoundary.tsx (TypeScript)

Code before:
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 });
  };

  componentDidCatch(error: Error, info: object) {
    const { dispatch } = this.props;

    // for invalid path segment errors
    if (error instanceof NotFoundError) {
      this.setState({
        type: '404',
      });
    }

Review comment (suggestion):
If you provide `ErrorBoundaryState` as a second generic argument to `React.Component` then this explicit typing is not needed. 
```suggestion
  initialState = {
```

Code after:
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) {
    const { dispatch } = this.props;

    // for invalid path segment errors
    if (error instanceof NotFoundError) {
      this.setState({
        type: '404',
      });
    }

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