[FIX] eslint warnings #1133
[FIX] eslint warnings #1133
Conversation
legalsylvain
requested review from
GabbasovDinar,
angelinaanaki and
victor-champonnois
|
Hi @geomer198, @robyf70, @CetmixGitDrone, @GabbasovDinar, |
| @@ -16,9 +16,6 @@ odoo.define("pos_product_multi_barcode.db", function (require) { | |||
| var res = this._super(products); | |||
| var self = this; | |||
|
|
|||
| if (!products instanceof Array) { | |||
| products = [products]; | |||
| } | |||
There was a problem hiding this comment.
sorry not a Javascript expert here, but are you sure this can be simply removed like that?
There was a problem hiding this comment.
super is called before, and the original function does the job. So it is not possible to not have an array at this step or did I missed something ?
https://github.com/odoo/odoo/blob/16.0/addons/point_of_sale/static/src/js/db.js#L192C1-L194C1
There was a problem hiding this comment.
Mmm, I'd say it isn't mutable
There was a problem hiding this comment.
Ow Thanks ! I didn't know. So what is the good way here ? Add an eslint exceptiont ?
There was a problem hiding this comment.
Well, I'm wrong and is indeed mutable as is part of the arguments object: https://eslint.org/docs/latest/rules/no-param-reassign
There was a problem hiding this comment.
So it is OK like that. Right ?
yes sorry I didn't see the job was done already in super.
There was a problem hiding this comment.
Strict mode code doesn’t sync indices of the arguments object with each parameter binding. Therefore, this rule is not necessary to protect against arguments object mutation in ESM modules or other strict mode functions.
I wouldn't remove it. According to that it's only mutable if running old code that hasn't been flagged with "use strict";
As we're slowly migrating to ESM, we can't rely on the argument being mutable (which is a bad thing, solved in "modern js")
There was a problem hiding this comment.
Thanks @ivantodorovich I didn't get that nuance :) Wouldn't the lint rule be a bit misguiding being that virtually all the code is run in strict mode?
There was a problem hiding this comment.
Indeed, since we already have the strict rule, it seems we could remove the no-param-reassign one
legalsylvain
force-pushed
the
16.0-FIX-warning-no-shadow
branch
from
65ff05d to
1f6aa17
Compare
Indeed ! typo fixed in commits texts. |
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at a47eedf. Thanks a lot for contributing to OCA. ❤️ |
Rational : for the time being, when we want to contribute to the OCA 16.0 branch and if we run pre-commit run -a, a lot of warning are poluting the log. So it's not easy to see if there is a real problem.
This PR fix all warnings. (a commit per type)
Thanks for your review !
CC : @OCA/pos-maintainers