Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: A trailing comma is not permitted after the rest element (tsx) #3528

Closed
rosskevin opened this issue Dec 19, 2017 · 2 comments
Closed
Labels
lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker.

Comments

@rosskevin
Copy link

rosskevin commented Dec 19, 2017

Prettier 1.9.1
Playground link

Input:

import * as React from 'react';
import * as PropTypes from 'prop-types';
import * as classNames from 'classnames';
import Transition from 'react-transition-group/Transition';
import { ClassNameMap } from '../styles/withStyles';
import { TransitionProps } from '../transitions/transition';

export type RippleClassKey =
  | 'root'
  | 'wrapper'
  | 'wrapperLeaving'
  | 'wrapperPulsating'
  | 'ripple'
  | 'rippleVisible'
  | 'rippleFast';

export interface RippleProps extends TransitionProps {
  classes: ClassNameMap<RippleClassKey>;
  pulsate?: boolean;
  rippleSize?: number;
  rippleX?: number;
  rippleY?: number;
}

interface State {
  rippleVisible: boolean;
  rippleLeaving: boolean;
}

/**
 * @ignore - internal component.
 */
class Ripple extends React.Component<RippleProps, State> {
  static defaultProps = {
    pulsate: false,
  };
  state: State = {
    rippleVisible: false,
    rippleLeaving: false,
  };

  handleEnter = () => {
    this.setState({
      rippleVisible: true,
    });
  };

  handleExit = () => {
    this.setState({
      rippleLeaving: true,
    });
  };

  render() {
    const {
      classes,
      className: classNameProp,
      pulsate,
      rippleX,
      rippleY,
      rippleSize,
      ...other,
    } = this.props;
    const { rippleVisible, rippleLeaving } = this.state;

    const className = classNames(
      classes.wrapper,
      {
        [classes.wrapperLeaving]: rippleLeaving,
        [classes.wrapperPulsating]: pulsate,
      },
      classNameProp,
    );

    const rippleClassName = classNames(classes.ripple, {
      [classes.rippleVisible]: rippleVisible,
      [classes.rippleFast]: pulsate,
    });

    const rippleStyles = {
      width: rippleSize,
      height: rippleSize,
      top: -(rippleSize / 2) + rippleY,
      left: -(rippleSize / 2) + rippleX,
    };

    return (
      <Transition onEnter={this.handleEnter} onExit={this.handleExit} {...other}>
        <span className={className}>
          <span className={rippleClassName} style={rippleStyles} />
        </span>
      </Transition>
    );
  }
}

(Ripple as any).propTypes = {
  /**
   * Useful to extend the style applied to components.
   */
  classes: PropTypes.object.isRequired,
  /**
   * @ignore
   */
  className: PropTypes.string,
  /**
   * If `true`, the ripple pulsates, typically indicating the keyboard focus state of an element.
   */
  pulsate: PropTypes.bool,
  /**
   * Diameter of the ripple.
   */
  rippleSize: PropTypes.number,
  /**
   * Horizontal position of the ripple center.
   */
  rippleX: PropTypes.number,
  /**
   * Vertical position of the ripple center.
   */
  rippleY: PropTypes.number,
};

export default Ripple;

Output:

SyntaxError: A trailing comma is not permitted after the rest element (62:15)
  60 |       rippleY,
  61 |       rippleSize,
> 62 |       ...other,
     |               ^
  63 |     } = this.props;
  64 |     const { rippleVisible, rippleLeaving } = this.state;
  65 | 

Expected behavior:
When I run prettier on a tsx file, it does not add a comma to the end or a rest element. Regression of #507 it seems, the only potential difference I see is that this is a tsx.

If I delete the comma manually then re-run prettier, it adds the comma back.

trailingCommas is currently all, but I tried with es5 and it made no difference.

@ikatyang
Copy link
Member

You have to specify --parser typescript on the playground.

playground

Prettier 1.9.2
Playground link

--parser typescript

Input:

import * as React from 'react';
import * as PropTypes from 'prop-types';
import * as classNames from 'classnames';
import Transition from 'react-transition-group/Transition';
import { ClassNameMap } from '../styles/withStyles';
import { TransitionProps } from '../transitions/transition';

export type RippleClassKey =
  | 'root'
  | 'wrapper'
  | 'wrapperLeaving'
  | 'wrapperPulsating'
  | 'ripple'
  | 'rippleVisible'
  | 'rippleFast';

export interface RippleProps extends TransitionProps {
  classes: ClassNameMap<RippleClassKey>;
  pulsate?: boolean;
  rippleSize?: number;
  rippleX?: number;
  rippleY?: number;
}

interface State {
  rippleVisible: boolean;
  rippleLeaving: boolean;
}

/**
 * @ignore - internal component.
 */
class Ripple extends React.Component<RippleProps, State> {
  static defaultProps = {
    pulsate: false,
  };
  state: State = {
    rippleVisible: false,
    rippleLeaving: false,
  };

  handleEnter = () => {
    this.setState({
      rippleVisible: true,
    });
  };

  handleExit = () => {
    this.setState({
      rippleLeaving: true,
    });
  };

  render() {
    const {
      classes,
      className: classNameProp,
      pulsate,
      rippleX,
      rippleY,
      rippleSize,
      ...other,
    } = this.props;
    const { rippleVisible, rippleLeaving } = this.state;

    const className = classNames(
      classes.wrapper,
      {
        [classes.wrapperLeaving]: rippleLeaving,
        [classes.wrapperPulsating]: pulsate,
      },
      classNameProp,
    );

    const rippleClassName = classNames(classes.ripple, {
      [classes.rippleVisible]: rippleVisible,
      [classes.rippleFast]: pulsate,
    });

    const rippleStyles = {
      width: rippleSize,
      height: rippleSize,
      top: -(rippleSize / 2) + rippleY,
      left: -(rippleSize / 2) + rippleX,
    };

    return (
      <Transition onEnter={this.handleEnter} onExit={this.handleExit} {...other}>
        <span className={className}>
          <span className={rippleClassName} style={rippleStyles} />
        </span>
      </Transition>
    );
  }
}

(Ripple as any).propTypes = {
  /**
   * Useful to extend the style applied to components.
   */
  classes: PropTypes.object.isRequired,
  /**
   * @ignore
   */
  className: PropTypes.string,
  /**
   * If `true`, the ripple pulsates, typically indicating the keyboard focus state of an element.
   */
  pulsate: PropTypes.bool,
  /**
   * Diameter of the ripple.
   */
  rippleSize: PropTypes.number,
  /**
   * Horizontal position of the ripple center.
   */
  rippleX: PropTypes.number,
  /**
   * Vertical position of the ripple center.
   */
  rippleY: PropTypes.number,
};

export default Ripple;

Output:

import * as React from "react";
import * as PropTypes from "prop-types";
import * as classNames from "classnames";
import Transition from "react-transition-group/Transition";
import { ClassNameMap } from "../styles/withStyles";
import { TransitionProps } from "../transitions/transition";

export type RippleClassKey =
  | "root"
  | "wrapper"
  | "wrapperLeaving"
  | "wrapperPulsating"
  | "ripple"
  | "rippleVisible"
  | "rippleFast";

export interface RippleProps extends TransitionProps {
  classes: ClassNameMap<RippleClassKey>;
  pulsate?: boolean;
  rippleSize?: number;
  rippleX?: number;
  rippleY?: number;
}

interface State {
  rippleVisible: boolean;
  rippleLeaving: boolean;
}

/**
 * @ignore - internal component.
 */
class Ripple extends React.Component<RippleProps, State> {
  static defaultProps = {
    pulsate: false
  };
  state: State = {
    rippleVisible: false,
    rippleLeaving: false
  };

  handleEnter = () => {
    this.setState({
      rippleVisible: true
    });
  };

  handleExit = () => {
    this.setState({
      rippleLeaving: true
    });
  };

  render() {
    const {
      classes,
      className: classNameProp,
      pulsate,
      rippleX,
      rippleY,
      rippleSize,
      ...other
    } = this.props;
    const { rippleVisible, rippleLeaving } = this.state;

    const className = classNames(
      classes.wrapper,
      {
        [classes.wrapperLeaving]: rippleLeaving,
        [classes.wrapperPulsating]: pulsate
      },
      classNameProp
    );

    const rippleClassName = classNames(classes.ripple, {
      [classes.rippleVisible]: rippleVisible,
      [classes.rippleFast]: pulsate
    });

    const rippleStyles = {
      width: rippleSize,
      height: rippleSize,
      top: -(rippleSize / 2) + rippleY,
      left: -(rippleSize / 2) + rippleX
    };

    return (
      <Transition
        onEnter={this.handleEnter}
        onExit={this.handleExit}
        {...other}
      >
        <span className={className}>
          <span className={rippleClassName} style={rippleStyles} />
        </span>
      </Transition>
    );
  }
}

(Ripple as any).propTypes = {
  /**
   * Useful to extend the style applied to components.
   */
  classes: PropTypes.object.isRequired,
  /**
   * @ignore
   */
  className: PropTypes.string,
  /**
   * If `true`, the ripple pulsates, typically indicating the keyboard focus state of an element.
   */
  pulsate: PropTypes.bool,
  /**
   * Diameter of the ripple.
   */
  rippleSize: PropTypes.number,
  /**
   * Horizontal position of the ripple center.
   */
  rippleX: PropTypes.number,
  /**
   * Vertical position of the ripple center.
   */
  rippleY: PropTypes.number
};

export default Ripple;

@ikatyang ikatyang added lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker. labels Dec 19, 2017
@rosskevin
Copy link
Author

Fantastic - thanks @ikatyang - we are trying to switch a codebase to typescript and still had parser: babylon.

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 6, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker.
Projects
None yet
Development

No branches or pull requests

2 participants