review changes#3
review changes#3rev-doshi merged 9 commits intoamaanbs:wdio_percy_support_v8from rev-doshi:wdio_percy_support_v8
Conversation
This reverts commit 0ba8af2. "removed # prefix for private fields"
| ) { | ||
| this._isAppAutomate = isAppAutomate | ||
| if (!_percyAutoCaptureMode || !['click', 'auto', 'screenshot', 'manual', 'testcase'].includes(_percyAutoCaptureMode as string)) { | ||
| if (_percyAutoCaptureMode && !_percyAutoCaptureMode || !CAPTURE_MODES.includes(_percyAutoCaptureMode as string)) { |
There was a problem hiding this comment.
Won't the first two conditions always evaluate to false?
There was a problem hiding this comment.
No, its correct, the variable is a string and it is null check, have tested it
There was a problem hiding this comment.
I mean the first two conditions are _percyAutoCaptureMode && !_percyAutoCaptureMode. It's like a & !a which always evaluates to false. So is that condition even necessary?
There was a problem hiding this comment.
It does always evaluate to undefined or false
But it is acting a null check for the next expression ->
CAPTURE_MODES.includes(_percyAutoCaptureMode as string)
There was a problem hiding this comment.
Nope. It's not. It's evaluating to (a && b) || c. How is the above condition guaranteeing null check on a?
| @@ -1,13 +1,14 @@ | |||
| declare namespace WebdriverIO { | |||
| import PercyCaptureMap from '../Percy/PercyCaptureMap.js' | |||
There was a problem hiding this comment.
https://www.typescriptlang.org/docs/handbook/namespaces-and-modules.html
namespaces/modules are used to organise code and declarations, import statement outside doesn't make sense and hence the build will fail too if we are to import outside
There was a problem hiding this comment.
I couldn't find any reference in the above link that says imports should be inside namespace declaration. What exact build error does it give if we do the imports outside?
Proposed changes
Types of changes
Checklist
Further comments
Reviewers: @webdriverio/project-committers