-
Notifications
You must be signed in to change notification settings - Fork 228
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
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.