chore: enable eslint:no-shadow #4089
Conversation
| @@ -24,10 +24,10 @@ import { isVisibleToScreenReaders } from '../../commons/dom'; | |||
| * @memberof checks | |||
| * @return {Mixed} True if aria-errormessage references an existing element that uses a supported technique. Undefined if it does not reference an existing element. False otherwise. | |||
| */ | |||
| function ariaErrormessageEvaluate(node, options, virtualNode) { | |||
| export default function ariaErrormessageEvaluate(node, options, virtualNode) { | |||
There was a problem hiding this comment.
What is the motivation behind this change?
There was a problem hiding this comment.
Follows our coding guidelines.
If you encounter any code that we maintain that does not put the default export at the top, you should update the file to do so.
There was a problem hiding this comment.
What is the motivation behind putting the default export at the top?
There was a problem hiding this comment.
Ensures that when you open the file the main code path is the first thing you read.
| const ids = cells | ||
| .filter(cell => cell.getAttribute('id')) | ||
| .map(cell => cell.getAttribute('id')); |
There was a problem hiding this comment.
This is much easier to read/understand 👍
| * } | ||
| */ | ||
|
|
||
| const mergeCheckLocale = (a, b) => { |
There was a problem hiding this comment.
Just curious - when is a function declaration preferred over a function expression? Also why is one preferred over the other?
There was a problem hiding this comment.
As far as I know we have no standard for when to use either. Mostly it's been up to the person writing the code.
There was a problem hiding this comment.
Generally I much prefer named functions, as they make debugging a bit easier.
There was a problem hiding this comment.
Both functions created using expressions and declarations have a name.
Welcome to Node.js v16.14.0.
Type ".help" for more information.
> function foo(){}
undefined
> foo.name
'foo'
> const bar = () => {}
undefined
> bar.name
'bar'
>
Co-authored-by: Stephen Mathieson <571265+stephenmathieson@users.noreply.github.com>
| * } | ||
| */ | ||
|
|
||
| const mergeCheckLocale = (a, b) => { |
There was a problem hiding this comment.
Generally I much prefer named functions, as they make debugging a bit easier.
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
This came up in a recent pr so went ahead and did it (hurray for resolving some 111 issues). For every file I touched I did the following:
const/letreduceto better array functions (when the reduce caused the name conflict)/* global */eslint stringsIn some files that had naming conflict issues the issues were in nested functions so I moved the function outside the nested scope.