Working around "Avoid leaking state in ember objects" ESLint rule

1

I often write components that use Objects or Arrays as configuration:

import Component from '@ember/component';
import { Styled, group } from 'ember-cli-ui-components';

export default Component.extend(Styled, {

  styles: {
    base: 'p-3 text-4 border-solid border-l-4 leading-normal',
    defaultStyle: 'info margined',

    types: group({
      info: 'bg-washed-blue border-blue text-dark-blue',
      error: 'bg-washed-red border-red text-dark-red',
      success: 'bg-washed-green border-green text-dark-green',
      warning: 'bg-light-yellow border-gold text-black-80'
    }),

    margins: group({
      margined: 'mb-4',
      marginless: ''
    }),
  }

});

In Ember 3.1 I’m getting hit with the new ESlint rule about “Avoiding leaking state in ember objects”.

I understand I can set properties in init(), but doing that drastically hampers the ergonomics of these sorts of APIs – so much so that I noticed Ember’s own properties that use this pattern are ignored by the rule. (And for good reason! I’d hate to define actions in an init method…)

My question is, is there a path forward here for these types of APIs, which have been laid out by Ember itself and I’m sure are used heavily by other Ember developers? I believe static class fields might be the answer? In the mean time, what are some things other folks are doing to work around this rule? And if everyone is ignoring it, should it be enabled by default?

2

We force everyone to wrap it in a computed property:

export default Component.extend(Styled, {
  styles: computed(function() { 
    return { ... };
  })
});

But yeah, I think you’re right that the long-term solution is to switch to ES6 classes and class fields. Since it’s basically syntactic sugar for setting values in the constructor, it doesn’t have the same issues with sharing state between instances.

3

A simple solution here would be to freeze the objects that you set this way. I find that to be an effective way to indicate to the linting rule that these properties will not be mutated at runtime (and therefore are not memory leak risks) without sacrificing the readability in the file.

For example:

import Component from '@ember/component';
import { Styled, group } from 'ember-cli-ui-components';

export default Component.extend(Styled, {

  styles: Object.freeze({
    base: 'p-3 text-4 border-solid border-l-4 leading-normal',
    defaultStyle: 'info margined',

    types: group({
      info: 'bg-washed-blue border-blue text-dark-blue',
      error: 'bg-washed-red border-red text-dark-red',
      success: 'bg-washed-green border-green text-dark-green',
      warning: 'bg-light-yellow border-gold text-black-80'
    }),

    margins: group({
      margined: 'mb-4',
      marginless: ''
    }),
  })
});

^ still reads better to me than the computed(function() {}) solution, avoids the linting rule, and makes the intent pretty clear (“don’t change these values at runtime”)…

9 Likes
4

Love this! Thanks Robert!

Just as a follow-up, is the general plan to move APIs like this (including things like Ember’s actions hash) to use class fields when we get them?

1 Like
5

Does this also work for arrays? I tend to do this pattern a lot:

const OPTIONS = [
  { code: 'foo', label: 'Foo' },
  { code: 'bar', label: 'Bar' },
  { code: 'baz', label: 'Baz' }
];

export default EmberObject.extend({
  options: OPTIONS,
  ...
});
1 Like
6

I believe the “real” path forward here is with classes and decorators.

Take a look at this example (which works with today’s ember versions):

export default class ClassNameDemoComponent extends Component {
  @className boundField = 'default-class';

  // With provided true/false class names
  @className('active', 'inactive') isActive = true;

  @className
  @computed
  get boundComputed() {
    // return generated class
  }

  @action
  foo() {
    // do something
  }
}

The documentation site for Ember-decorators is awesome (:clap: @pzuraq ) and has lots more details:

http://ember-decorators.github.io/ember-decorators/latest/

2 Likes
7

What is the difference (conceptually) between actions and just functions (methods) on the class?

Is it just to offer some name-spacing so at a glance you know if a method is private to the component or is intended to be called from the template?

8

@sukima the original reason was to avoid conflicts between action names and common class hooks, such as destroy, click, etc. It also was important before closure actions were a thing, when actions had to be sent manually and you had to do things to maintain context. Now that we have closure actions, actions can be bound directly which is more similar to how other frameworks do it.

9

I actually disable that rule. I don’t encounter that leaking state as I always make my objects/arrays immutable.

10

The most recent time I hit this on a component, I switched to a real ECMA class with a property initializer and was pleasantly surprised that it all just worked (once I turned on the necessary Babel plugin and updated my eslint config to use the Babel parser).

11

Did you do this via ember-decorators?

12

I didn’t need any, it was a simple component. But if it had CPs, yeah, I would have needed that.

13

Yes. It’s exactly the same as the earlier example. I did something like this:

myProperty: Object.freeze([
    { foo: "bar" },
    { bar: "foo" }
])