Refactor utils modules before migrating to TS. #3870
Conversation
Status
|
garg3133
force-pushed
the
utils-refactor
branch
from
eeb3770 to
ffe0fa8
Compare
lib/utils/logger/log_settings.js
Outdated
| } | ||
| class LogSettings { | ||
| constructor() { | ||
| this.__outputEnabled = true; |
There was a problem hiding this comment.
Instead of __outputEnabled, we should use #outputEnabled.
Ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields
There was a problem hiding this comment.
I have no problem with either notations, it's just that __ notation is what we're using everywhere in the Nightwatch project, and because we are anyway going to convert this file to TS just after this PR is merged where we'll use the private keyword, using/not using # doesn't really matter.
There was a problem hiding this comment.
Done. Needed to update eslint to work with # as well. But not sure if # is supported in Node.js 16, let's see.
There was a problem hiding this comment.
Okay, it seems that they are available since Node.js 12, but I wonder why it was only implemented in ECMAScript 2022?
Co-authored-by: Vaibhav Singh <singh.vaibhav2011@gmail.com>
Co-authored-by: Vaibhav Singh <singh.vaibhav2011@gmail.com>
… into utils-refactor
| @@ -3,7 +3,7 @@ | |||
| "eslint:recommended" | |||
| ], | |||
| "parserOptions": { | |||
| "ecmaVersion": 2020, | |||
| "ecmaVersion": 13, | |||
There was a problem hiding this comment.
I couldn't find 13 in the documentation. We can use latest to use the latest environment parser in ESLint.
Ref: https://eslint.org/docs/latest/use/configure/language-options#specifying-environments
There was a problem hiding this comment.
13 is present in inline documentation, which is same as setting it to 2022.
I didn't specify latest because earlier also 2020 version was mentioned instead of latest and I'm not sure of the reason for that, but I suppose it's so that any changes in ESLint do not affect us, as long as we don't explicitly decide to move to a new version ourselves.
There was a problem hiding this comment.
@beatfactor Can you please look into this once, if it is fine to use latest or should we just keep to a specific version?
Also, please review and merge this PR afterwards, as it is a blocker for migrating the utils/logger module.
No description provided.