search

Vauxoo / pylint-odoo

1 stars 4 forks source link

[REF] eslint: Mute [W7903(javascript-lint)- Unexpected 'this'. [Error/no-invalid-this] #70

Closed hugho-ad closed 8 years ago

hugho-ad commented 8 years ago

The lint is not compatible with the features added with jquery since is necesary the use of $() in order to get the jquery feautures. http://stackoverflow.com/questions/1051782/jquery-this-vs-this

http://eslint.org/docs/rules/no-invalid-this

hugho-ad commented 8 years ago

@oscarolar what do you think?

moylop260 commented 8 years ago

Thanks @hugho-ad I have created the following PR https://github.com/Vauxoo/pylint-odoo/pull/71 to change it if @oscarolar is agree.

NOTE: Feel free of create PR to silent checks of js here

oscarolar commented 8 years ago

@hugho-ad can you point me at specific cases where you want to disable the this?, the rule says that: Under the strict mode, this keywords outside of classes or class-like objects might be undefined and raise a TypeError. which for me is valid and should stay.

hugho-ad commented 8 years ago

@oscarolar see the next example: https://github.com/Vauxoo/yoytec/blob/8.0/website_yoytec/static/src/js/collapse_descriptions.js#L7

and this travis log https://travis-ci.com/Vauxoo/yoytec/jobs/48047574

oscarolar commented 8 years ago

There are workarounds to fix the use of this, I think is a good rule, the line you are pointing can be fixed passing the event handler and reffering to his target like this:

      $('.corner').click(function(event){
          var target = $(event.target).prev('div');
      ....

By keeping it enabled it would mean a lot of work when checks become fully active.

The reason you post that we "need it" for JQuery $() is not valid because we can access to it via the callback/chaining function on its signature, On the other hand using this makes the code look cleaner and allows to use less variables in the code by just calling the anonymous functions without params.

I vote for disabling it.