avoid-leaking-state-in-ember-objects is showing for ember-cli-mirage Factory objects

[BillyRayPreachersSon]

I've just upgraded ember-cli from 2.17.0 to 2.18.0, and haver fixed most of the new linting errors (which are really informative, btw - thanks for that).

However, I'm seeing ember/avoid-leaking-state-in-ember-objects being flagged on an object that isn't an Ember object, and thus can't be fixed using the suggested workaround.

The class in question is an ember-cli-mirage Factory. Looking at the source code, it doesn't extend Ember's CoreObject, but is instead defined as:

let Factory = function() {
    ...
};

In addition, those Factories don't have an init() method, and nor does Factory.extend set any _super property, so section in the docs for fixing ember/avoid-leaking-state-in-ember-objects that show how to fix this wouldn't apply to these Factories.

We're using ember-cli-mirage 0.2.1, but the latest source code (0.4.1) shows exactly the same issues.

Is there any way these errors can be removed globally for non-Ember objects, but kept for other Ember objects in the test folders, without having to add overrides on a file-by-file basis? Perhaps isEmberObject can be beefed up to check for more than just an extend method?

Thanks.

[rwjblue]

Sorry you are having troubles here! TBH, I don't think we can do much more to detect (statically) if a given import is going to be an Ember object or not other than what we are already doing. Basically, if something "duck types" like an Ember object, we consider it one...

I think the best solution here would be to add an additional entry to overrides in your .eslintrc.js to disable this rule (and possibly others that may also have false positives). That might look like:

// default .eslintrc.js for ember-cli@2.18

module.exports = {
  root: true,
  parserOptions: {
    ecmaVersion: 2017,
    sourceType: 'module'
  },
  plugins: [
    'ember'
  ],
  extends: [
    'eslint:recommended', 
    'plugin:ember/recommended'
  ],
  env: {
    browser: true
  },
  rules: {
  },
  overrides: [
    // node files
    {
      files: [
        'index.js',
        'testem.js',
        'ember-cli-build.js',
        'config/**/*.js',
        'tests/dummy/config/**/*.js'
      ],
      parserOptions: {
        sourceType: 'script',
        ecmaVersion: 2015
      },
      env: {
        browser: false,
        node: true
      }
    },

    // test files
    {
      files: ['tests/**/*.js'],
      excludedFiles: ['tests/dummy/**/*.js'],
      env: {
        embertest: true
      }
    },

    // mirage files
    {
      files: ['mirage/**'],
      rules: {
        'ember/avoid-leaking-state-in-ember-objects': 'off'
      }
    }
  ]
};

[rwjblue]

Once we have figured out the right subset of overrides to use for mirage/** we should probably open a PR to add that info to the README / website / etc so others can discover it too...

[BillyRayPreachersSon]

Hi @rwjblue,

Thank for the advice on disabling the rule for Mirage files. Sadly, it didn't work.

I added it to both <project-root>/.eslintrc.js and <project-root>/tests/.eslintrc.js, but I get exactly the same error:

<project-root>/mirage/mirage/factories/request.js
  28:3  error  Only string, number, symbol, boolean, null, undefined, and function are allowed as default properties  ember/avoid-leaking-state-in-ember-objects

I tried changing the excluded path from ['mirage/**'] to ['mirage/factories/*.js'], but get the same problem.

After that, I tried adding "ember/avoid-leaking-state-in-ember-objects": "off" to the top-level rules section in both files, but still the error comes.

This is really hampering my ability to upgrade Ember to 2.18, as my build keeps erroring when the code isn't at fault. Right now, I don't care if I can disable this rule globally, but it seems that I'm unable to do that (or don't know the correct syntax to do so).

Here's my <project-root>/.eslintrc.js file - where am I going wrong?

Thanks

module.exports = {
  root: true,
  parserOptions: {
    ecmaVersion: 2017,
    sourceType: 'module'
  },
  plugins: [
    'ember'
  ],
  extends: [
    'eslint:recommended',
    'plugin:ember/recommended'
  ],
  env: {
    browser: true
  },
  globals: {
    "server": false,
    "document": false,
    "window": false,
    "-Promise": false
  },
  rules: {
    "no-cond-assign": [
      2,
      "except-parens"
    ],
    "curly": 2,
    "no-debugger": 0,
    "eqeqeq": 2,
    "no-eval": 2,
    "guard-for-in": 0,
    "wrap-iife": 0,
    "linebreak-style": 0,
    "new-cap": 0,
    "no-caller": 2,
    "no-empty": 0,
    "no-new": 0,
    "no-plusplus": 0,
    "no-undef": 2,
    "dot-notation": 0,
    "strict": 0,
    "no-eq-null": 2,
    "no-unused-vars": 2
  },
  overrides: [
    // node files
    {
      files: [
        'testem.js',
        'ember-cli-build.js',
        'config/**/*.js'
      ],
      parserOptions: {
        sourceType: 'script',
        ecmaVersion: 2015
      },
      env: {
        browser: false,
        node: true
      }
    },

    // test files
    {
      files: ['tests/**/*.js'],
      excludedFiles: ['tests/dummy/**/*.js'],
      env: {
        embertest: true
      }
    },

    // mirage files
    {
      files: ['mirage/**'],
      rules: {
        'ember/avoid-leaking-state-in-ember-objects': 'off'
      }
    }
  ]
};

[BillyRayPreachersSon]

Aah - I seem to have found a workaround.

I can define my object property data outside of the class, and reference it inside. As the linting is probably only looking for curly braces or square brackets inside the class, this will pass as it finds neither of those.

So, changing this:

import { Factory } from 'ember-cli-mirage';

export default Factory.extend({
    aProperty: {
        foo: 42,
        bar: ['some', 'things']
    }
});

to this:

import { Factory } from 'ember-cli-mirage';

let aPropertyData = {
    foo: 42,
    bar: ['some', 'things']
};

export default Factory.extend({
    aProperty: aPropertyData
});

fixes the problem for me with no disabling of linting rules required.

[rwjblue]

Hmmm, my suggestion should generally work... 🤔

Can you push up a demo app that I can help debug?

[BillyRayPreachersSon]

I've moved on to debugging a very different problem at the moment, and as I have a workaround for the linting issue (in my previous comment), it's unlikely that I'll be given any time to knock up a demo app.

If I do, I'll let you know.

Thanks

[Dhaulagiri]

Hmmm, my suggestion should generally work...

I think it's because the mirage folder is under tests/dummy/. I used this and it worked:

// mirage files
    {
      files: ['tests/dummy/mirage/**'],
      rules: {
        'ember/avoid-leaking-state-in-ember-objects': 'off'
      }
    }

[rwjblue]

I think it's because the mirage folder is under tests/dummy/.

Ya, in an addon that is definitely correct. I didn't catch that this issue was RE: an addon.

[BobrImperator]

[QUESTION]
Is it intended for

  overrides: [
    // node files
    // override rules for serializers
    {
      files: [
        'index.js',
        'testem.js',
        'ember-cli-build.js',
        'config/**/*.js',
        'tests/dummy/config/**/*.js',
        'serializers/*.js',
      ],
      parserOptions: {
        sourceType: 'script',
        ecmaVersion: 2017,
      },
      env: {
        browser: true,
      },
      rules: {
        'ember/avoid-leaking-state-in-ember-objects': 'off',
      },
    },
  ],

and especially this

      files: [
        'index.js',
        'testem.js',
        'ember-cli-build.js',
        'config/**/*.js',
        'tests/dummy/config/**/*.js',
        'serializers/*.js',
      ],

to skip all index.js files without paying attention to where they are nested?
I mean, that when sourceType has value 'script', linter thinks that my
users/index.js is not a module, but script, therefore it doesn't allow for import export keywords

Parsing error: 'import' 'export' may appear only with 'sourceType: module'

I wouldn't have noticed that if I didn't copy pasted @rwjblue example.
and of course, when I change back to sourceType: 'module' everything's fine.

Although it's probably not even regarding ember, but eslint instead, but it's worth a note I guess.
I'm sorry in advance if something is not right in my post, I'm new to the business 🔢