-
-
Notifications
You must be signed in to change notification settings - Fork 23
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 allowThis to rxjs-no-unsafe-scope rule #52
Comments
I'll have to look into this - if you pardon the pun. I have vague memories about intending to have the rule allow for method calls via I'm curious though, would your |
The Maybe something like:
|
Are you absolutely sure that it's failing for method calls via I'm inclined to separate |
hmm... I will need time to double confirm this. Middle of another project now. Will update here once I re-check (in a few days). Thanks for the quick response. |
No worries. In the interim, I will update the fixtures to include non-failing expectations that demonstrate/document the rule's behaviour with method calls. |
See 6d3aa9c for the added expectations. Note that the method calls don't effect failures. Once we've figured out whether or not this is the behaviour you are seeing, I'll happily add |
I did additional tests. With this code:
This is my findings:
With these findings, I am actually not sure if |
The The rule allows method calls, but I think an I'm surprised that read-only use in The primary motivator for the rule was to flag the use of variables at outer scopes, but usage of The reason for allowing method calls was for things like AngularFire - where you might call a function that returns a database observable to which you want to switch or whatever. Or a HTTP service with which you want to call I think having |
The other thing that I will say is that the rules is this package don't necessarily reflect what "best practice" is. You might want to use some rules but not others. Each rule's default configuration isn't necessarily representative of "best practice", either. I've spoken with Ben Lesh about incorporating a subset of the rules in this package into another package to reflect what the RxJS core team considers to be best practice, but we've not yet taken that further. |
Thanks for the clarifications on Agree that Look forward to having |
Yeah, it would pass. Mutating state within a method isn't something I intend to catch with the rule. Well, not right now, anyway. |
4.7.0 has been published and it includes the |
Tried using 4.7.0. Works great. Just sharing one minor odd observation (in my code/usage). Since I am linting test code using the same config, the I decided to Cheers! |
Yes, if there are places in which you don't want the rule enforced, the approach that you've taken is the way to go. Any of TSLint's comment-based flags should be able to be applied - e.g. |
I am using observables in methods within classes. Some of the functions do things like:
With
rxjs-no-unsafe-scope
, it says "Unsafe scopes are forbidden".I think it is due to
this.doSomething()
. Would be great if we canallowThis
(and have it enabled by default)...just likeallowParameters
.The text was updated successfully, but these errors were encountered: