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

Add new rule no-leaking-state-in-program-scope #447

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MichalBryxi
Copy link

@MichalBryxi MichalBryxi commented Jul 3, 2019

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
@bmish
Copy link
Member

bmish commented Aug 6, 2019

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.

@MichalBryxi
Copy link
Author

@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
Copy link
Author

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.

@bmish bmish changed the title no-leaking-state-in-program-scope Add new rule no-leaking-state-in-program-scope Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leaking state in program scope
2 participants