Giter Site home page Giter Site logo

eslint-config-molindo's People

Contributors

amannn avatar dependabot[bot] avatar googol7 avatar msparer avatar symn avatar

Stargazers

avatar

Watchers

avatar avatar avatar avatar avatar avatar

Forkers

amannn googol7

eslint-config-molindo's Issues

Evaluate `react/no-unstable-nested-components`

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-unstable-nested-components.md

I was a bit unsure which of these patterns is preferable for a longer time:

function LinkButton({href, ...rest}) {
  const Component = useMemo(() =>
    function Link(props) {
      return <a href={href} {...props} />
    },
    [href]
  );

  return <Button as={Component} {...rest} />
}
function LinkButton({href, ...rest}) {
  return <Button as="a" href={href} {...rest} />
}

… the second one has the advantage that the component remains mounted for the lifetime of the component. I was concerned however about handling an unknown prop like href within Button.

After discovering react-polymorphic-types I'm now largely in favour of the second approach. I'm not sure yet if it's worth linting for, but we could consider it.

I'd set allowAsProps to true in any case to not be too restrictive.

Add `no-use-before-define`

This can be tricky: A variable defined with const can be compiled via Babel to a regular var and therefore a violation of this rule might not throw in a browser. However if this code isn't compiled and run directly with modern tools where const is natively supported, a ReferenceError will be thrown.

https://eslint.org/docs/rules/no-use-before-define

Use declaration style for functions

With the introduction of React Hooks I began to think about what the most readable React component looks like.

With classes we had different syntax for properties and methods:

class Button {
  property = 'foo';

  getProperty() {
    return this.property;
  }
}

If we write React components as functions I think it becomes a bit harder to see at a glance what are functions and what are properties, if you'd declare both of them with expressions.

const property = 'foo'
const getProperty = () => property;

In a reduced example this is usually not an issue, but a larger component might make this less obvious.

React components themself are also closely related to functions, therefore I think the function keyword makes more sense for them. Also you can use the export statement on the same line as the declaration, which isn't the case with expressions. Furthermore I think function is semantically more correct to const (is it a constant?).

Therefore I'd go for this in the future:

function Button(props) {
  const property = 'foo';

  function getProperty() {
    return property;
  }

 // …
}

Arrow functions are only allowed for callbacks, potentially handling the topic of this correctly there.

I propose these two new rules here:

"func-style": ["error", "declaration", {"allowArrowFunctions": false}],
"prefer-arrow-callback": "error",

Valid:

function declaration(foo) {
  return foo;
}

useEffect(() => {
  document.title = 'foo';
});

Invalid:

const expressionWithArrowFunction = foo => foo;

const expressionWithAnonymousFunction = function(foo) {
  return foo;
};

useEffect(function () {
  document.title = 'foo';
});

useEffect(function setDocumentTitle() {
  document.title = 'foo';
});

Other advantages (from discussion below):

  • It's more explicit about return values.
  • In case there's a direct return, it's later easier to add a body, also temporarily to add logging. This also avoids changing the indent of the returned expression in regards to git diffs.
  • I'd assume the function declaration syntax is more beginner friendly – the updated React docs also suggest them.

Note that prefer-arrow-callback is auto-fixable, but func-style can be codemodded.

Use node for import resolving

The current resolver config needs a webpack config file which might not be available in every project. The only feature that we use from webpack is to provide src as an additional module directory.

We could achieve this by using node as well:

  "import/resolver": {
      "node": {
        "paths": [
          "node_modules",
          "src"
        ]
      }
    }

That would work in projects without webpack as well.

Consider turning `react/display-name` off

It often has false positives like:

t('questions.description', {
  contact: (children) => (
    <Link href="/contact">
      <a>{children}</a>
    </Link>
  )
})

With the patterns we use to define React components this should usually not be an issue.

Disable `import/export` for TypeScript

This creates false positives when there are overloads.

E.g.:

import useQueryParamString from './useQueryParamString';

export default function useQueryParamNumber(
  name: string,
  required: true
): number;
export default function useQueryParamNumber(
  name: string,
  required?: false
): number | undefined;
export default function useQueryParamNumber(name: string, required?: any) {
  const value = useQueryParamString(name, required);
  return value ? parseFloat(value) : undefined;
}

Also allow tests in subdirectories

In a subdirectory
src/utils/tests/subdir/something-test.js
I get this error:

import sinon from „sinon"

'sinon' should be listed in the project's dependencies, not devDependencies.

In the root level it works without problems:
src/utils/tests/semething-test.js
import sinon from „sinon“

This is because of:

   'import/no-extraneous-dependencies': [
     ERROR,
     {
       devDependencies: [
         'webpack.config.js',
         '**/__testUtils__/**/*.js',
         '**/__tests__/*-test.js'
       ]
     }
   ],

Allow for/of loops

There are valid cases for this, especially with async/await. I think the lint rule doesn't help here anymore, as we're quite familiar with different iteration methods and this only creates noise.

Allow spaced-comment for TypeScript type imports

TypeScript has this special syntax:

/// <reference types="jest" />

We should configure spaced-comment to allow this:

    'spaced-comment': [
      'error',
      'always',
      {
        line: {
          markers: ['/'],
          exceptions: ['/']
        }
      }
    ]

Consider `member-ordering` in `.tsx` files.

The background is that react/sort-prop-types currently doesn't work for TypeScript prop types.

If .tsx files are exclusively used for React components, a workaround could be to be more strict in those files and apply typescript/member-ordering there.

Note that this will have some false positives, but it might be a pragmatic solution.

I'm not sure if this is a good idea though. A lot of codebases use .tsx for all TypeScript files, regardless if they contain JSX or not. When in doubt, we should maybe leave this turned off.

Move peer dependencies to dependencies

This should be blocked by eslint/eslint#3458, however I just found this option on the CLI:

  --resolve-plugins-relative-to path::String  A folder where plugins should be
                                              resolved from, CWD by default

Maybe with that we could work around the issue.

Allow for/of iteration

I think it's usually a good idea to favour iteration based on array methods (map, filter, …) but for/of can be handy if you need async/await or break.

Consider more test file patterns

The extension .e2e.{js,jsx,ts,tsx} would be helpful. Cypress also introduced .cy.{js,jsx,ts,tsx} recently with component tests.

Remove no-lonely-if

I think there are valid cases where the code is easier to read if you have a lonely if/else.

E.g.

if (conditionA) {
  ...
} else {
  if (conditionB) {
    ...    
  } else {
    ...        
  }
}

Explicit length checks

Code like this can be confusing:

const isValidEnterSubmit = isEnter && searchResults.length;

If there are results and you console.log(isValidEnterSubmit) you might see a number like 12 as the last value in the condition is assigned to the variable. Explicit length checks like searchResults.length > 0 avoid this issue and is therefore preferred.

Also in JSX this is problematic:

<div>{searchResults.length && searchResults[0].title}</div>

This will render 0 if there are no results, as numbers are emitted to the DOM, but booleans are not.

Theres a rule for this in an external plugin, but I'd like to avoid adding yet another plugin – at least as long as consumers of this package have to install plugins separately.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.