-
Notifications
You must be signed in to change notification settings - Fork 237
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: adds no jest import rule #95
feat: adds no jest import rule #95
Conversation
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.
LGTM
docs/rules/no-jest-import.md
Outdated
@@ -0,0 +1,20 @@ | |||
# Disallow importing Jest(no-jest-import) | |||
|
|||
The jest object is automatically in scope within every test file. The methods in |
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.
Use a code block for the jest
object.
docs/rules/no-jest-import.md
Outdated
|
||
The jest object is automatically in scope within every test file. The methods in | ||
the jest object help create mocks and let you control Jest's overall behavior. | ||
It is therefore completely unnecessary to import in Jest, as Jest doesn't export |
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.
I would change Jest to jest
here, as you refer to the object.
docs/rules/no-jest-import.md
Outdated
|
||
This rule reports on any importing of Jest. | ||
|
||
To name a few: *var jest = require('jest'); *const jest = require('jest'); |
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.
Maybe use code blocks here?
var jest = require('jest');
const jest = require('jest');
import jest from 'jest';
import {jest as test} from 'jest';
docs/rules/no-jest-import.md
Outdated
To name a few: *var jest = require('jest'); *const jest = require('jest'); | ||
*import jest from 'jest'; *import {jest as test} from 'jest'; | ||
|
||
There is no correct usage of this code, other than to not import Jest it in the |
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.
Jest => jest
rules/no-jest-import.js
Outdated
if (node.source.value === 'jest') { | ||
context.report({ | ||
node, | ||
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`, |
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.
The message can be moved into a variable so you won't write it twice.
index.js
Outdated
@@ -29,6 +30,7 @@ module.exports = { | |||
'jest/no-disabled-tests': 'warn', | |||
'jest/no-focused-tests': 'error', | |||
'jest/no-identical-title': 'error', | |||
'jest/no-jest-import': 'error', |
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.
technically breaking to add it here, mind removing it?
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.
See inline comments
@ranyitz thanks for reviewing! 😀 |
88413a0
to
52b3494
Compare
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.
Thanks!
Closes #90.
Adds the eslint rule 'no-jest-import', disallowing any import from Jest.
Adds tests and documentation for rule.