eslint-plugin-jest icon indicating copy to clipboard operation
eslint-plugin-jest copied to clipboard

feat: prefer importing jest globals [new rule]

Open MadeinFrance opened this issue 1 year ago • 2 comments

  • Solves #1101

Issue: https://github.com/jest-community/eslint-plugin-jest/issues/1101

Prefer importing Jest globals (prefer-importing-jest-globals)

🔧 This rule is automatically fixable by the --fix CLI option.

This rule aims to enforce explicit imports from @jest/globals.

  1. This is useful for ensuring that the Jest APIs are imported the same way in the codebase.
  2. When you can't modify Jest's injectGlobals configuration property, this rule can help to ensure that the Jest globals are imported explicitly and facilitate a migration to @jest/globals.

Rule details

Examples of incorrect code for this rule

/* eslint jest/prefer-importing-jest-globals: "error" */

describe('foo', () => {
  it('accepts this input', () => {
    // ...
  });
});

Examples of correct code for this rule

/* eslint jest/prefer-importing-jest-globals: "error" */

import { describe, it } from '@jest/globals';

describe('foo', () => {
  it('accepts this input', () => {
    // ...
  });
});

Further Reading

image

MadeinFrance avatar Jan 16 '24 15:01 MadeinFrance

Is it possible to add an autofix for this? I.e. detecting what globals are used, and add either an import or require of them?

I think just flagging global usage can be done by not having jest/globals in the env config of ESLint, so if we add this rule, I think it should provide value above and beyond just marking them.

SimenB avatar Jan 17 '24 08:01 SimenB

Is it possible to add an autofix for this? I.e. detecting what globals are used, and add either an import or require of them?

I think just flagging global usage can be done by not having jest/globals in the env config of ESLint, so if we add this rule, I think it should provide value above and beyond just marking them.

Hi @SimenB, I've added an auto fixer in the logic 👨🏻‍🔧

MadeinFrance avatar Jan 22 '24 17:01 MadeinFrance

@G-Rath can we move forward to Main with this PR? Most of the clean up is done.

MadeinFrance avatar Apr 02 '24 11:04 MadeinFrance

@MadeinFrance I'm waiting to see what's happening with our upcoming major whichll probably get released this week.

This PR looks good to me so you can consider it off your plate - I'll handle landing it and doing any revising that might be needed :)

G-Rath avatar Apr 02 '24 17:04 G-Rath

Thanks again @MadeinFrance! I have done some minor refactoring that I recommend checking out if you're interested but don't feel bad about any of it - you did the grunt work which I've just built on and I've been working with these rules for years so never expected a first-time contributor to have gotten my changes on their first rule.

The fixer is still a bit rough but I'm happy to improve that in follow-up PRs since you've already gone through a lot here and I think the roughness is just with very unlikely edge cases e.g.

  • import jest from '@jest/globals' should give import jest, { ... } rather than import { jest, ... }
  • we should retain template literals if they're used in the require
  • we shouldn't end up with the next line being dedented after adding the import
  • ideally we should include data in the test errors
  • ideally we could probably support settings.jest.globalAliases too (this is super low priority though)

G-Rath avatar Apr 06 '24 21:04 G-Rath

:tada: This PR is included in version 28.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar Apr 06 '24 21:04 github-actions[bot]

@G-Rath I'm thrilled to see this landing on the release line. Thank you for your review and patience 🙏 Seeing how this PR has grown since the initial commit is amazing.

I will use this in our corporate repos and improve the edge cases if required.

MadeinFrance avatar Apr 07 '24 18:04 MadeinFrance