eslint-functional / eslint-plugin-functional

ESLint rules to disable mutation and promote fp in JavaScript and TypeScript.
MIT License
856 stars 30 forks source link

compatibility with react, functional-parameters ? #76

Closed iangregsondev closed 4 years ago

iangregsondev commented 4 years ago

Hi,

First of all, great little addition to eslint! Trying to use ramba (looked at immutablejs but settled on ramba in the end) - I needed a way to ensure I was doing any mutable operations - and this library seems to nail it ! Thanks!

One thing, I decided to implement the

 "plugin:functional/lite"

I was wondering if the order matters, I have it placed here, just before the prettier extends.

  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/eslint-recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:functional/lite",
    "prettier/@typescript-eslint",
    "prettier"
  ],

After enabling the lite version I noticed that it was complaining where I was using parameterless functions - react has many ( every component could potentially be a parameterless functions) i.e.

export const App = () => { // This is a component, it takes no params (props)
  // const location = useLocation()

So I did an override, and it seems to have fixed it. Am I missing anything else?

    "functional/functional-parameters": [
      "error",
      {
        "enforceParameterCount": false
      }
    ],

Thanks once again!

RebeccaStevens commented 4 years ago

Your config all seems correct 😄.

With regard to functional-parameters enforcing a parameter count, maybe we should disabled this by default for the lite config. @jonaskello, What do you have think about this?

jonaskello commented 4 years ago

Yes, that might be a good idea. Also for people looking just to disable mutation perhaps there should be an `immutable' preset or something like that.

iangregsondev commented 4 years ago

Like the idea of immutable preset. Actually this is my case. Just interested in immutability and not a pure functional direction.

Although I am using ramda 😀

RebeccaStevens commented 4 years ago

Which rules currently enabled in the lite config would we want to disable in a new immutable config?

Note: we already have the no-mutations rules set.

iangregsondev commented 4 years ago

@RebeccaStevens I am probably not the best person to comment on that as I have only just started using it.

I haven't come into other issues so far - but truthfully I have only just scratched the surface :-)

iangregsondev commented 4 years ago

I thought I would give an update on my findings and what i am doing.

SO I am now extending from this

    "plugin:functional/no-mutations",

as I am only really interested in the no mutations stuff.

I also setup the following

    "functional/immutable-data": [
      "error",
      {
        "ignorePattern": ["^mutable_"]
      }
    ],
    "functional/prefer-readonly-type": [
      "error",
      {
        "ignoreInterface": true,
        "allowLocalMutation": true
      }
    ],

and remove the complaints about mutable_

    "@typescript-eslint/camelcase": [
      "error",
      {
        "allow": ["^mutable_"]
      }
    ],

Maybe this is not the final version :-) ALthough it seems to be working now.

Just one question, I was forced to use allowLocalMutations but I don't fully understand it. I did read the docs. What is classed as local state ?

Here is an example (its using redux). Redux needs to mutate things. The following line causes issues

MyNiceNewSlitEntity[]

If I add a readonly into the generic i.e.

Payload<readonly MyNiceNewSlitEntity[]>

The error goes away but then

mutable_state.entities can't be assigned becasue the action.payload is readonly. The only way I got this to work was using allowLocalMutation, but I don't understand what I have enabled / disabled :-(

I checked created a local const array and pushing values onto it and it doesn't allow me - so that hasn't broken. Any ideas ?

    getMyNiceNewSlitSuccess: (mutable_state, action: PayloadAction<MyNiceNewSlitEntity[]>) => {
      mutable_state.loaded = true
      mutable_state.entities = action.payload
    },

here is the interface which is used for the array

export interface MyNiceNewSlitEntity {
  id: number
}
RebeccaStevens commented 4 years ago

allowLocalMutation for the prefer-readonly-type rule simply makes the rule ignore locally defined types. Here locally defined basically means defined inside a function. So for example:

// This typedef will violate the rule as x is mutable.
type Point = {
  x: number;
  readonly y: number;
}

function foo() {
  // This typedef is local so it is ok with `allowLocalMutation`.
  type Point = {
    x: number;
    readonly y: number;
  }

  // ...
}

prefer-readonly-type only checks type definitions, it doesn't check if data is being mutated; the immutable-data rule checks for that.

If you provide me with a more detailed example or a link to your code I'll be able to assist more with set up for your particular use case.

With regard to the original issue, I'm going to close it now (feel free to keep commenting here for help with your set up). I'll open a new issue for the thing mentioned above.

RebeccaStevens commented 4 years ago

@jonaskello maybe we should set up a spectrum community?

iangregsondev commented 4 years ago

Thanks @RebeccaStevens , that answers my question.

jonaskello commented 4 years ago

@RebeccaStevens I have not yet used spectrum but it seems like a nice alternative to slack, gitter etc. :-). I was thinking this plugin could be a channel in a larger eslint community but could not find any community for eslint. So I set up a separate community for this plugin here and added a badge to the readme. Please go join :-).