@W-11446192@: checks if any files are processed by both eslint and eslint-lwc and emits warning #782
Conversation
gkjung
force-pushed
the
d/W-11446192
branch
4 times, most recently
from
03cab58 to
3bcb517
Compare
messages/DefaultRuleManager.js
Outdated
| "targetSkipped": "Target: '%s' was not processed by any engines.", | ||
| "targetsSkipped": "Targets: '%s' were not processed by any engines." | ||
| "targetsSkipped": "Targets: '%s' were not processed by any engines.", | ||
| "pathDoubleProcessed": "A file was processed by both ESLint and ESLint-LWC, which may result in duplicate violations. Check out our documentation on target patterns to cuztomize which paths each engine should target.", |
There was a problem hiding this comment.
In each of these, "cuztomize" should be "customize".
src/lib/DefaultRuleManager.ts
Outdated
| if (pathsDoubleProcessed.length === 1) { | ||
| uxEvents.emit(EVENTS.WARNING_ALWAYS, messages.getMessage('warning.pathDoubleProcessed')); | ||
| } else if (pathsDoubleProcessed.length > 1) { | ||
| uxEvents.emit(EVENTS.WARNING_ALWAYS, messages.getMessage('warning.pathsDoubleProcessed')); | ||
| } |
There was a problem hiding this comment.
@gkjung Could you help understand - I feel like I'm missing something obvious - if the warning messages are going to be the same, why do we need an if/else? Can't we do a >=1 check?
Also, can we include the file names that got analyzed by both engines? A comma-separated string with contents of pathsDoubleProcessed would help users create a cleaner target pattern.
There was a problem hiding this comment.
Ah, got it, singular vs plural.
If we were to include the file names, would it be possible to keep it a single message?
There was a problem hiding this comment.
That would be cleaner, I'm not sure the best way to phrase the message to suit both. What if the first sentence was At least one file was processed by both ESLint and ESLint-LWC, which may result in duplicate violations.?
There was a problem hiding this comment.
I thought about including the file names, but if they had several I wasn't sure how to do that without the message getting really long. Any ideas?
There was a problem hiding this comment.
Good point. How about we put a limit on number of files included in the message? We could list out up to 10 file paths/names and conditionally add and more if the list has more remaining that we didn't cover.
I like the sentence structure. @teresa-allen-sfdc Please let us know your thoughts.
There was a problem hiding this comment.
This is what it's currently looking like WARNING: At least one file was processed by both ESLint and ESLint-LWC simultaneously, which could result in duplicate violations. Customize targetPatterns properties for eslint and eslint-lwc engines in /Users/grace.jung/.sfdx-scanner/Config-pilot.json to remove overlap on the following file(s): /Users/grace.jung/Desktop/sfdx-scanner/test/code-fixtures/invalid-lwc/invalidApiDecorator/noLeadingUpperCase.js, /Users/grace.jung/Desktop/sfdx-scanner/test/code-fixtures/projects/dep-test-app/folder-a/SomeGenericFile.js, /Users/grace.jung/Desktop/sfdx-scanner/test/code-fixtures/projects/dep-test-app/folder-a/jquery-3.1.0.js, /Users/grace.jung/Desktop/sfdx-scanner/test/code-fixtures/projects/dep-test-app/folder-b/AnotherGenericFile.js, /Users/grace.jung/Desktop/sfdx-scanner/test/code-fixtures/projects/dep-test-app/folder-b/jquery-3.5.1.js, /Users/grace.jung/Desktop/sfdx-scanner/test/code-fixtures/projects/dep-test-app/folder-c/Burrito.js, and 26 more. @teresa-allen-sfdc @rmohan20 How does this look? It only require one message for plural/singular and I listed out 5 because it was getting really long with 10
There was a problem hiding this comment.
@gkjung This looks great! I especially like the and 26 more. We could even stick to just two or three to drive the point across.
There was a problem hiding this comment.
I think three makes sense, and if they want to see the rest they can always fix those and rerun if there's any still overlapping.
gkjung
force-pushed
the
d/W-11446192
branch
4 times, most recently
from
1a16a63 to
42454bb
Compare
…lint-lwc and emits warning
If any of the same paths are processed by eslint and eslint-lwc, we emit a warning to the user that they should use target patterns in the config file.