fix(prefer-importing-jest-globals): don't add imports in the middle of statements

[ejzimmer]

The current fixer implementation for prefer-importing-jest-globals inserts the import directly before the reporting node. This works fine when the reporting node is a function call.

describe(...)

results in

import { describe } from 'jest/globals';
describe(...)

But it breaks when the reporting node is part of a variable declaration

const onClick = jest.fn();

results in

const onClick = import { jest } from '@jest/globals';
jest.fn();

This change updates the fixer so new imports are inserted at the top of the file.

console.log('foo');
const onClick = jest.fn();

results in

import { jest } from '@jest/globals';
console.log('foo')
const onClick = jest.fn();

Existing imports are unchanged

console.log('foo');
import { describe } from '@jest/globals';
describe('the thing', () => {
  const onClick = jest.fn();
}

results in

console.log('foo');
import { describe, jest } from '@jest/globals';
describe('the thing', () => {
  const onClick = jest.fn();
}

Resolves #1634

[G-Rath]

could you undo all the unneeded indenting changes in the test cases please?

[ejzimmer]

could you undo all the unneeded indenting changes in the test cases please?

Oh yikes, yeah, clearly prettier did not have my back.

[G-Rath]

this looks fine, though I'd appreciate if you could cleanup the intending on the new cases (sorry, Prettier doesn't handle javascript-in-javascript blocks 😅) and explain a bit more about

Why didn't I just put the import at the top in both cases? Because it would have broken a couple of tests, eg:

I don't follow how it would have broken the test you've mentioned

[ejzimmer]

Why didn't I just put the import at the top in both cases? Because it would have broken a couple of tests, eg:

I don't follow how it would have broken the test you've mentioned

That test places the require inside a function, and I just wasn't sure if that was important. Like, is requiring different modules inside of different functions something people do with commonjs modules? (I work almost exclusively with esmodules where obviously that just isn't a thing)

[G-Rath]

What matters for us (mostly) is if its syntactically valid e.g. if we would end up duplicating a const in the same scope, that'd be bad; technically rules don't need to care about style because those that care should have lint rules that style anyway e.g. we don't need to bother figuring out if we should be using double or single quotes, or about placing semicolons.

We still do some of that stuff when it's easy because it can still be nice to do, but if a particular thing is very complex then we can choose not to take on that complexity.

In this case though I think putting it at the top of the file is the right move as it's likely we'll have multiple references throughout nested functions, though I think we should be fine to just always put the require at the very top of the file like you're doing for imports (but it's probably not the end of the world?)

[github-actions]

🎉 This issue has been resolved in version 28.8.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀