-
-
Notifications
You must be signed in to change notification settings - Fork 357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add prefer-event-key
rule
#226
Merged
Merged
Changes from 29 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
1a69094
Adds preliminary implementation for the rule
ankeetmaini 364c6be
Improves code and handles destructuring as well.
ankeetmaini d6b4931
Adds more test
ankeetmaini c640d6e
Adds fixer code
ankeetmaini 8709cd6
Resolves rebase left overs correctly.
ankeetmaini bb608e0
Uses String.fromCharCode instead of keeping a map.
ankeetmaini 62a432b
Adds another test and meta.type
ankeetmaini 4634daa
Fixes only in case of equality checks
ankeetmaini fbbb841
Remove trailing blanks
futpib a339099
Add a failing test
futpib 97f597f
Addresses review comments.
ankeetmaini 608c085
Removed utils function in favour of top-level helpers
ankeetmaini 6ddc7aa
Finalizes approach!
ankeetmaini 3e91f68
Fixes fixer code wherein it was fixing anything under an if condition
ankeetmaini ec24ffa
Removes VS Code settings.json
ankeetmaini cb0e100
Update prefer-key-over-key-code.md
sindresorhus ed61fa5
Renames rule to `prefer-event-key`
ankeetmaini 15a326c
Removes extra citation and passes tests
ankeetmaini 7034784
Adds tests for keys and tests fail. :(
ankeetmaini ccdaa2f
Handles when event object is directly destructured.
ankeetmaini b9268da
Adds more keys and tests
ankeetmaini f6e19a3
Removes rebase errors
ankeetmaini 3b193ed
Update prefer-event-key.js
sindresorhus ce645f2
Update prefer-event-key.js
sindresorhus 2854a3b
Update readme.md
sindresorhus 8b31b2b
Update prefer-event-key.md
sindresorhus 2c37990
Update readme.md
sindresorhus cb3d413
Update readme.md
sindresorhus 78d640d
Update prefer-event-key.md
sindresorhus 6a95bb2
Merge branch 'master' into prefer-key
ankeetmaini d3f0b38
Adds more checks
ankeetmaini 631a2a9
Update rule documentation
ankeetmaini 9dafef5
Update prefer-event-key.md
sindresorhus File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode` | ||
|
||
Enforces the use of [`KeyboardEvent#key`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key) over [`KeyboardEvent#keyCode`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode) which is deprecated. The `.key` property is also more semantic and readable. | ||
|
||
|
||
## Fail | ||
|
||
```js | ||
window.addEventListener('keydown', event => { | ||
console.log(event.keyCode); | ||
}); | ||
``` | ||
|
||
```js | ||
window.addEventListener('keydown', event => { | ||
if (event.keyCode === 8) { | ||
console.log('Backspace was pressed'); | ||
} | ||
}); | ||
``` | ||
|
||
|
||
## Pass | ||
|
||
```js | ||
window.addEventListener('click', event => { | ||
console.log(event.key); | ||
}); | ||
``` | ||
|
||
```js | ||
window.addEventListener('keydown', event => { | ||
if (event.key === 'Backspace') { | ||
console.log('Backspace was pressed'); | ||
} | ||
}); | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,222 @@ | ||
'use strict'; | ||
const getDocsUrl = require('./utils/get-docs-url'); | ||
|
||
const keys = [ | ||
'keyCode', | ||
'charCode', | ||
'which' | ||
]; | ||
|
||
// https://github.com/facebook/react/blob/b87aabd/packages/react-dom/src/events/getEventKey.js#L36 | ||
// Only meta characters which can't be deciphered from `String.fromCharCode()` | ||
const translateToKey = { | ||
8: 'Backspace', | ||
9: 'Tab', | ||
12: 'Clear', | ||
13: 'Enter', | ||
16: 'Shift', | ||
17: 'Control', | ||
18: 'Alt', | ||
19: 'Pause', | ||
20: 'CapsLock', | ||
27: 'Escape', | ||
32: ' ', | ||
33: 'PageUp', | ||
34: 'PageDown', | ||
35: 'End', | ||
36: 'Home', | ||
37: 'ArrowLeft', | ||
38: 'ArrowUp', | ||
39: 'ArrowRight', | ||
40: 'ArrowDown', | ||
45: 'Insert', | ||
46: 'Delete', | ||
112: 'F1', | ||
113: 'F2', | ||
114: 'F3', | ||
115: 'F4', | ||
116: 'F5', | ||
117: 'F6', | ||
118: 'F7', | ||
119: 'F8', | ||
120: 'F9', | ||
121: 'F10', | ||
122: 'F11', | ||
123: 'F12', | ||
144: 'NumLock', | ||
145: 'ScrollLock', | ||
186: ';', | ||
187: '=', | ||
188: ',', | ||
189: '-', | ||
190: '.', | ||
191: '/', | ||
219: '[', | ||
220: '\\', | ||
221: ']', | ||
222: '\'', | ||
224: 'Meta' | ||
}; | ||
|
||
const isPropertyNamedAddEventListener = node => | ||
node && | ||
node.callee && | ||
node.callee.property && | ||
node.callee.property.name === 'addEventListener'; | ||
|
||
const getEventNodeAndReferences = (context, node) => { | ||
const eventListener = getMatchingAncestorOfType(node, 'CallExpression', isPropertyNamedAddEventListener); | ||
const callback = eventListener && eventListener.arguments && eventListener.arguments[1]; | ||
switch (callback && callback.type) { | ||
case 'ArrowFunctionExpression': | ||
case 'FunctionExpression': { | ||
const eventVariable = context.getDeclaredVariables(callback)[0]; | ||
const references = eventVariable && eventVariable.references; | ||
return { | ||
event: callback.params && callback.params[0], | ||
references | ||
}; | ||
} | ||
|
||
default: | ||
return {}; | ||
} | ||
}; | ||
|
||
const isPropertyOf = (node, eventNode) => { | ||
return ( | ||
node && | ||
node.parent && | ||
node.parent.object && | ||
node.parent.object === eventNode | ||
sindresorhus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
}; | ||
|
||
// Third argument is a condition function, as in passed to `Array#filter()` | ||
sindresorhus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Helpful if nearest node of type also needs to have some other property | ||
const getMatchingAncestorOfType = (node, type, fn = n => n || true) => { | ||
let current = node; | ||
while (current) { | ||
if (current.type === type && fn(current)) { | ||
return current; | ||
} | ||
|
||
current = current.parent; | ||
} | ||
|
||
return null; | ||
}; | ||
|
||
const getParentByLevel = (node, level) => { | ||
let current = node; | ||
while (current && level) { | ||
level--; | ||
current = current.parent; | ||
} | ||
|
||
if (level === 0) { | ||
return current; | ||
} | ||
}; | ||
|
||
const fix = node => fixer => { | ||
// Since we're only fixing direct property access usages, like `event.keyCode` | ||
const nearestIf = getParentByLevel(node, 3); | ||
if (!nearestIf || nearestIf.type !== 'IfStatement') { | ||
return; | ||
} | ||
|
||
const {right = {}, operator} = nearestIf.test; | ||
const isTestingEquality = operator === '==' || operator === '==='; | ||
const isRightValid = isTestingEquality && right.type === 'Literal' && typeof right.value === 'number'; | ||
// Either a meta key or a printable character | ||
const keyCode = translateToKey[right.value] || String.fromCharCode(right.value); | ||
// And if we recognize the `.keyCode` | ||
if (!isRightValid || !keyCode) { | ||
return; | ||
} | ||
|
||
// Apply fixes | ||
return [ | ||
fixer.replaceText(node, 'key'), | ||
fixer.replaceText(right, `'${keyCode}'`) | ||
]; | ||
}; | ||
|
||
const create = context => { | ||
const report = node => { | ||
context.report({ | ||
message: `Use \`key\` instead of \`${node.name}\``, | ||
sindresorhus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
node, | ||
fix: fix(node) | ||
}); | ||
}; | ||
|
||
return { | ||
'Identifier:matches([name=keyCode], [name=charCode], [name=which])'(node) { | ||
// Normal case when usage is direct -> `event.keyCode` | ||
const {event, references} = getEventNodeAndReferences(context, node); | ||
if (!event) { | ||
return; | ||
} | ||
|
||
const isPropertyOfEvent = Boolean(references && references.find(r => isPropertyOf(node, r.identifier))); | ||
if (isPropertyOfEvent) { | ||
report(node); | ||
} | ||
}, | ||
|
||
Property(node) { | ||
// Destructured case | ||
const propertyName = node.value && node.value.name; | ||
if (!keys.includes(propertyName)) { | ||
return; | ||
} | ||
|
||
const {event, references} = getEventNodeAndReferences(context, node); | ||
if (!event) { | ||
return; | ||
} | ||
|
||
const nearestVariableDeclarator = getMatchingAncestorOfType( | ||
node, | ||
'VariableDeclarator' | ||
); | ||
const initObject = | ||
nearestVariableDeclarator && | ||
nearestVariableDeclarator.init && | ||
nearestVariableDeclarator.init; | ||
|
||
// Make sure initObject is a reference of eventVariable | ||
const isReferenceOfEvent = Boolean( | ||
references && references.find(r => r.identifier === initObject) | ||
); | ||
if (isReferenceOfEvent) { | ||
report(node.value); | ||
return; | ||
} | ||
|
||
// When the event parameter itself is destructured directly | ||
const isEventParamDestructured = event.type === 'ObjectPattern'; | ||
if (isEventParamDestructured) { | ||
// Check for properties | ||
for (const prop of event.properties) { | ||
if (prop === node) { | ||
report(node.value); | ||
} | ||
} | ||
} | ||
} | ||
}; | ||
}; | ||
|
||
module.exports = { | ||
create, | ||
meta: { | ||
type: 'suggestion', | ||
docs: { | ||
url: getDocsUrl(__filename) | ||
}, | ||
fixable: 'code' | ||
} | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please describe that it's fixable and what is fixable. See the other docs for how to write it.