[ARGG-857] Add rule forbidding the use of axios library #656
Conversation
| }, | ||
| }); | ||
|
|
||
| ruleTester.run('no-moment', noAxios, { |
There was a problem hiding this comment.
haha let's change to no-axios, otherwise all looks good!
…lint-config-skyscanner into ARGG-857_add_no_axios_rule
| { | ||
| "main": "src/index.js", | ||
| "peerDependencies": { | ||
| "eslint": "^8.53.0" |
There was a problem hiding this comment.
this just means the eslint-config-skyscanner package needs to be on v8+, right?
There was a problem hiding this comment.
Yup. it's only used in the containing package. I would remove this requirement completely, but then eslint complains about a dependency that isn't listed (but is used in the rule)
| context.report(node, 'Deprecated require of axios package'); | ||
| } | ||
| }, | ||
| ImportDeclaration: (node) => { |
There was a problem hiding this comment.
very unlikely to happen but, does this cover dynamic imports? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import
There was a problem hiding this comment.
Added code to handle them. Had to bump the ES version to something that supports dynamic imports, left it at latest.
| }, | ||
| }); | ||
|
|
||
| ruleTester.run('no-axios', noAxios, { |
There was a problem hiding this comment.
I would also add a test for named imports and for requires with property access
require('axios').default
There was a problem hiding this comment.
Happy to add that. It should work though as the rule checks for the argument to require() call being axios - it doesn't care about what consumer does with result
There was a problem hiding this comment.
Added a few tests more
What:
Adds a rule forbidding the use of axios library. The rule name is
skyscanner-no-axios/no-axiosWhy:
Axios is known to leak sensitive details in it's error objects
How: