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

Option to allow mutation of this inside class constructors #16

Open
axefrog opened this issue Jul 29, 2016 · 2 comments
Open

Option to allow mutation of this inside class constructors #16

axefrog opened this issue Jul 29, 2016 · 2 comments

Comments

@axefrog
Copy link

axefrog commented Jul 29, 2016

I'm using immutable classes as clean, transportable and easy-to-debug-and-maintain wrappers around Immutable.js state objects in order to clearly define the shape of the underlying data. Such a class is constructed by passing it an instance of the state object in question. Operations which makes changes to state instead return a new instance of the class, constructed with the new state object created as a result of the operation.

Obviously this means I use:

{
  "immutable/no-let": 2,
  "immutable/no-this": 0,
  "immutable/no-mutation": 2
}

In the constructor, initialising the one-and-only class field this.state violates the "no-mutation" rule.

Because a constructor is an initialiser of sorts, in the same way that const a = b is initialising the value of a, if I allow use of this, then no-mutation should not apply in a class constructor.

I understand that this cannot be safely used for constructors defined via an ES3/5 function, given that such functions cannot be guaranteed to be used as a constructor, but for ES6 classes, the constructor is guaranteed to only be used during construction, and it can thus be inferred that we're initialising a new value (the class instance) and not mutating an existing value.

The workaround is adding an inline // eslint-disable-line immutable/no-mutation to every constructor, but it would be cleaner if this could be done via a single configuration option.

Example
Here is an example of how I am using classes functionally, rather than for OO:

import Immutable from 'immutable';

class TimeInterval
{
  constructor(state) {
    this._state = state; // eslint-disable-line immutable/no-mutation
  }

  static create(t, tSession) {
    return new TimeInterval(Immutable.Map({
      t,
      tOffset: t - tSession,
      changeSets: ChangeSetsMap.create().state
    }));
  }

  get state() {
    return this._state;
  }

  get time() {
    return this._state.get('t');
  }

  get timeOffset() {
    return this._state.get('tOffset');
  }

  get changeSets() {
    return new ChangeSetsMap(this._state.get('changeSets'));
  }

  getLatestGraph(batchType = 'granular') {
    const changeSets = this.changeSets[batchType];
    if(!changeSets) {
      throw new Error('Invalid changeset batch type: ' + batchType);
    }
    return changeSets.latestGraph;
  }

  updateGraph(updater) {
    const changeSets = this.changeSets
      .update('granular', list => list.update(updater))
      .update('cascading', list => list.update(updater))
      .update('single', list => list.update(updater));
    const state = this._state.set('changeSets', changeSets.state);
    return new TimeInterval(state);
  }
}
@deepu105
Copy link

Its really annoying when the rule triggers inside a constructor. May be it would be nice to have another rule to disable checking in constructor or even better to have rule option to exclude constructor or a pattern

@ljharb
Copy link

ljharb commented Apr 26, 2017

class MyArray extends Array { constructor() { this.length = 'mutated'; } } - you can never guarantee anything in the constructor isn't mutating an existing value or triggering a setter upstream.

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

No branches or pull requests

3 participants