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

[prefer-importing-jest-globals] autofix is broken #1634

Closed
ejzimmer opened this issue Aug 2, 2024 · 8 comments · Fixed by #1639 or #1645
Closed

[prefer-importing-jest-globals] autofix is broken #1634

ejzimmer opened this issue Aug 2, 2024 · 8 comments · Fixed by #1639 or #1645

Comments

@ejzimmer
Copy link
Contributor

ejzimmer commented Aug 2, 2024

The plugin is relying on the old version of the eslint config to check whether the code is using esmodules or commonjs.

const isModule = context.parserOptions.sourceType === 'module';

should be something like

const isModule =  context.parserOptions.sourceType === 'module' || context.languageOptions.sourceType === 'module';

There doesn't seem to be an easy workaround for this, as the old structure is invalid in the new version of eslint. (ie I can't just add parserOptions.sourceType to my config. eslint won't run if I do).

There also seems to be an issue with imports being inserted inline. If I change the plugin code to use the new version of the config, so it uses esmodule imports, I end up with a few cases similar to this:

<Form onSubmit={import { describe, expect, it, jest } from '@jest/globals';jest.fn()}>{({formProps}) => <form {...formProps}>{children}</form>}</Form>

where the import is inserted directly where it's used, rather than at the top of the file.

@SimenB
Copy link
Member

SimenB commented Aug 2, 2024

PR for const isModule thing would be highly appreciated :)

Not sure about the other one... Is it just in JSX it fails?

@ejzimmer
Copy link
Contributor Author

ejzimmer commented Aug 6, 2024

PR for const isModule thing would be highly appreciated :)

I've got a fix PR, but I keep getting remote: Permission to jest-community/eslint-plugin-jest.git denied to ejzimmer. when I try to push. Any ideas?

Not sure about the other one... Is it just in JSX it fails?

Once we've got this other fix in, I'll dig into it a little further

@G-Rath
Copy link
Collaborator

G-Rath commented Aug 6, 2024

I've got a fix PR, but I keep getting remote: Permission to jest-community/eslint-plugin-jest.git denied to ejzimmer. when I try to push. Any ideas?

You need to do your fix on a fork, and then do a pull request from your fork into our repository

@ejzimmer
Copy link
Contributor Author

ejzimmer commented Aug 6, 2024

I've got a fix PR, but I keep getting remote: Permission to jest-community/eslint-plugin-jest.git denied to ejzimmer. when I try to push. Any ideas?

You need to do your fix on a fork, and then do a pull request from your fork into our repository

Thanks!
#1639

Copy link

🎉 This issue has been resolved in version 28.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ejzimmer
Copy link
Contributor Author

The issue with the import being inserted mid-line

<Form onSubmit={import { describe, expect, it, jest } from '@jest/globals';jest.fn()}>{({formProps}) => <form {...formProps}>{children}</form>}</Form>

was still occurring after the fix above ^^ was released. I've got a PR to fix this here

@G-Rath should I open a new issue for it?

@G-Rath
Copy link
Collaborator

G-Rath commented Aug 30, 2024

@ejzimmer na no need for a new issue, though feel free to stick Resolves #1634 in your PR so that they're linked

We don't strictly need an issue for every PR :)

Copy link

github-actions bot commented Sep 4, 2024

🎉 This issue has been resolved in version 28.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment