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

Lint improvements #23

Open
aleksuk opened this issue Feb 1, 2018 · 3 comments
Open

Lint improvements #23

aleksuk opened this issue Feb 1, 2018 · 3 comments
Labels

Comments

@aleksuk
Copy link
Contributor

aleksuk commented Feb 1, 2018

  1. Don't forbid to add new line inside function brakets
    refs: https://github.com/airbnb/javascript#functions--signature-invocation-indentation

As Is

const template = _.template(trans('A site requires a domain if it is published. If you wish to remove ${name}, unpublish the site.'));

Allow To Do

const template = _.template(
  trans('A site requires a domain if it is published. If you wish to remove ${name}, unpublish the site.')
);
  1. Make possible to declare is already declared in the upper case variables for local scope
import {
  isAssigned,
} from 'src/ide/modules/collections/domains/compare';

const getState = (data, id) = {
  // lint says that "isAssigned" is already declared in upper scrope
  const isAssigned = data.assign;
  const isAssignedToCurrentSite = isAssigned && data.id === id;

  return { isAssigned, isAssignedToCurrentSite };
};

The same issue with react and redux with mapDispatchToProps.

import React from 'react';
import { openCustomDomainModal } from '../store/actions/action-creators';

// lint says that "openCustomDomainModal" is alreade declared in upper scrope
function MyComponent({ openCustomDomainModal }) {
  /* ... */
}
/* prop types, default props and etc */

const mapDispatchToProps = { openCustomDomainModal };
export default connect(null, mapDispatchToProps)(MyComponent);
  1. Setup correct work with chai. (take a look this plugin: https://www.npmjs.com/package/eslint-plugin-chai-friendly)
// linst says `Expected an assignment or function call`
expect(onPageChange).not.to.have.been.called;
@aleksuk
Copy link
Contributor Author

aleksuk commented Feb 1, 2018

Also next issue is available with react and redux, if you don't use destructuring assignment, lint doesn't see usage of some variables, or is it correct?

function Component(/*...*/) {
/*...*/
}

Component.propTypes = {
  /* ... */
  // linst says `PropTypes is defined but prop is never used
  autoClose: PropTypes.bool,
  // linst says `PropTypes is defined but prop is never used
  onAccept: PropTypes.func,
};
// uasge of autoClose and onAccept
const mapDispatchToProps = (dispatch, ownProps) => ({
  accept() {
    const { autoClose, onAccept } = ownProps;

    onAccept();

    if (autoClose) {
      dispatch(hideModal());
    }
  },
// ...
}

export default connect(null, mapDispatchToProps)(Component);

@aleksuk
Copy link
Contributor Author

aleksuk commented Feb 1, 2018

@RusinovAnton
Copy link
Contributor

linst says `PropTypes is defined but prop is never used

prop isn't used within component but in the mapDispatchToProps which is in fact is outside.

There are a bunch of related issues are filled in eslint-plugin-react repo: https://github.com/yannickcr/eslint-plugin-react/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+unused+prop

There is also this ticket airbnb/javascript#1089 about overriding default rules and no-unused-prop-types is one of often overridden.

So I think this rule should be turned off in the base config for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants