Immer: Usage with eslint no-param-reassign

Created on 14 Sep 2018  Â·  13Comments  Â·  Source: immerjs/immer

I don't want to disable the eslint rule "no-param-reassign" globally, because it prevents errors.

So this seems to be the best way to use immer with that rule enabled:

const nextState = produce(baseState, (draftState) => {
    /* eslint-disable no-param-reassign */
    draftState.test = "New value";
    /* eslint-enable no-param-reassign */
});

There is a way to specify "ignorePropertyModificationsFor" for that eslint rule (https://eslint.org/docs/rules/no-param-reassign). So draftState could be ignored. But that would only exclude that name.

Is there a better way without disabling that rule?

Most helpful comment

Just to make it quicker for ya'll,
if you are extending airbnb,
just do the following in your eslint.rc file:

module.exports = {
  extends: [
    'airbnb', // this means you have "props" = "true" by default
  ],
  rules: {
    'no-param-reassign': ['error', { props: true, ignorePropertyModificationsFor: ['draft'] }],
  },
};

All 13 comments

@brummelte I don't think your example is a param reassign and the rule shouldn't trigger on it

You are correct, I just copied it from your example. But the question stays the same:

const nextState = produce(baseState, (draftState) => {
    /* eslint-disable no-param-reassign */
    draftState.test = "New value";
    /* eslint-enable no-param-reassign */
});

That isn't a param reassign either, please test before reporting issues

Op vr 14 sep. 2018 20:17 schreef Jan Brummelte notifications@github.com:

You are correct, I just copied it from your example. But the question
stays the same:

const nextState = produce(baseState, (draftState) => {
/* eslint-disable no-param-reassign /
draftState.test = "New value";
/
eslint-enable no-param-reassign */
});

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/mweststrate/immer/issues/189#issuecomment-421442603,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhLyxkysvti5JdWlWYMvl6VlEFCjDks5ua_LSgaJpZM4Wppfh
.

It is with the option { "props": true }.

Which the airbnb preset uses: https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/best-practices.js

image

Wow that's an insane rule disabling half the language. But not going to
argue it, that is too tiring. you might try to set option
ignorePropertyModificationsFor: "draft".

Otherwise; drop either that rule or immer.

Op vr 14 sep. 2018 20:27 schreef Jan Brummelte notifications@github.com:

It is with the option { "props": false }.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/mweststrate/immer/issues/189#issuecomment-421445152,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhGhUuzs6ChESdpV98O5NiPxjqvZ7ks5ua_T6gaJpZM4Wppfh
.

For future readers: did that option solve the issue?

Op vr 14 sep. 2018 om 21:13 schreef Jan Brummelte <[email protected]

:

Closed #189 https://github.com/mweststrate/immer/issues/189.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/mweststrate/immer/issues/189#event-1846845687, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhMcz4mI1XoQ3aFsqDJZ66lGU-ys9ks5ua__vgaJpZM4Wppfh
.

I also don't like disabling it within its file or function scope.

I think // eslint-disable-line no-param-reassign, although ridiculously repetitive, would do the work.

For future readers: did that option solve the issue?

The issue was, that that is not the perfect solution. But yes, that setting would work.

Ok, to summarize:

  1. no-param-reassign rule makes perfect sense, I actually strongly recommend it, because draft = X assignments are really a no-op
  2. props: true option though kinda defeats the purpose of immer, and should be kept to false (the default), or an exception could be made by setting ignorePropertyModificationsFor: "draft"

Just to make it quicker for ya'll,
if you are extending airbnb,
just do the following in your eslint.rc file:

module.exports = {
  extends: [
    'airbnb', // this means you have "props" = "true" by default
  ],
  rules: {
    'no-param-reassign': ['error', { props: true, ignorePropertyModificationsFor: ['draft'] }],
  },
};

Also in case you have multiple producers in the same scope, like this:

  markAsSubmitted = isLoading => {
    this.setState(produce(draft => {
      draft.loading = isLoading;
      draft.secondLevel = draft.secondLevel.map(produce(sketch => {
        sketch.submitted = true;
        sketch.thirdLevel = sketch.thirdLevel.map(produce(outline => {
          outline.submitted = true;
        }));
      }));
    }));
  };

This rule would work:

    "no-param-reassign": ["error", {
      "props": true,
      "ignorePropertyModificationsFor": ["draft", "sketch", "outline"]
    }],

For people coming from future google searches, extending on @jaimefps comment above

To enable more semantic variable naming as well as avoiding problem in case of multiple producers in the same scope, we can do

module.exports = {
  extends: [
    'airbnb', // this means you have "props" = "true" by default
  ],
  rules: {
    'no-param-reassign': ['error', { props: true, ignorePropertyModificationsForRegex: ["^draft"] }],
  },
};

This should make it work for draft, draftPost, draftState

Another trick that works:

const nextState = produce(baseState, (draft) => {
    const tempDraft = draft;
    tempDraft .test = "New value";
});
Was this page helpful?
0 / 5 - 0 ratings

Related issues

CodingPapi picture CodingPapi  Â·  7Comments

martinkadlec0 picture martinkadlec0  Â·  3Comments

cakoose picture cakoose  Â·  6Comments

Knacktus picture Knacktus  Â·  5Comments

TrueWill picture TrueWill  Â·  4Comments