Skip to content
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

Merged
merged 2 commits into from
Mar 5, 2018

Conversation

brianlmacdonald
Copy link
Contributor

Closes #90.

Adds the eslint rule 'no-jest-import', disallowing any import from Jest.

Adds tests and documentation for rule.

Copy link
Contributor

@ranyitz ranyitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -0,0 +1,20 @@
# Disallow importing Jest(no-jest-import)

The jest object is automatically in scope within every test file. The methods in
Copy link
Contributor

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.


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
Copy link
Contributor

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.


This rule reports on any importing of Jest.

To name a few: *var jest = require('jest'); *const jest = require('jest');
Copy link
Contributor

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';

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jest => jest

if (node.source.value === 'jest') {
context.report({
node,
message: `Jest is automatically in scope. Do not import Jest, as it doesn't export anything.`,
Copy link
Contributor

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',
Copy link
Member

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?

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments

@SimenB
Copy link
Member

SimenB commented Mar 3, 2018

@ranyitz thanks for reviewing! 😀

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants