eslint-plugin-ember icon indicating copy to clipboard operation
eslint-plugin-ember copied to clipboard

Add new rule `no-leaking-state-in-program-scope`

Open MichalBryxi opened this issue 5 years ago • 3 comments

Consider following service:

var cachedFoo = null;

export default Service.extend({
  fetchFoo(count) {
    if(!cachedFoo) {
      fetch(`./api/?results=${count}`).then(function(response) {
        cachedFoo = response;
      });
    }
  }

  getFoo() {
    return cachedFoo;
  }
});

This service will probably work as expected in the scope of running Ember code.

But if there are two tests calling this service with different value for count when calling fetchFoo, then calls to getFoo will always return the response of the first test.

  • Basic implementation of this new rule
  • Trying to address. Fixes: #446

MichalBryxi avatar Jul 03 '19 22:07 MichalBryxi

I'm not sure that it's sufficient for the rule to simply look for var usage outside the Ember class. There could be plenty of legitimate code using var outside the Ember class.

To make this rule more targeted, perhaps it could look for assignments (x = 456;) inside the Ember class where the variable was originally declared var x = 123; outside the Ember class.

bmish avatar Aug 06 '19 05:08 bmish

@bmish sounds about right. The same applies to more complex structures, right?

// scope outside of Ember class
var x = { foo: 123 };

// scope inside Ember class
// this should throw error
x.foo = 456;

MichalBryxi avatar Aug 08 '19 06:08 MichalBryxi

Hmm, looking at how-to get to the scope of given variable and it does not seem like it's entirely easy. From what I've seen eslint-scope should be what I'm looking for to help me. If anyone has more experience with this, I'm more than happy to get any pointers.

MichalBryxi avatar Aug 08 '19 07:08 MichalBryxi