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

Feature Request: Make this plugin work with bun #1431

Closed
TkDodo opened this issue Sep 14, 2023 · 7 comments · Fixed by #1543
Closed

Feature Request: Make this plugin work with bun #1431

TkDodo opened this issue Sep 14, 2023 · 7 comments · Fixed by #1543
Labels

Comments

@TkDodo
Copy link

TkDodo commented Sep 14, 2023

bun had a 1.0 release recently, and a lot of things from its test runner work like jest. Mostly, it's a drop-in replacement to do:

- import { describe, expect, it } from '@jest/globals'
+ import { describe, expect, it } from 'bun:test'

however, changing the import like that makes the eslint-plugin-jest no longer work, because the rules only apply to imports from '@jest/globals', as we can see here:

// the identifier is imported from @jest/globals,
// so return the original import name
if (maybeImport.source === '@jest/globals') {

We are currently patching this plugin to also detect bun imports by extending this if statement:

if (maybeImport.source === '@jest/globals' || maybeImport.source === 'bun:test') { 

I'd be happy to hear your thoughts on making this work either out of the box, or making it configurable. I'm also up for contributing a PR.

Thanks 🙏

@SimenB
Copy link
Member

SimenB commented Sep 14, 2023

I don't wanna hard code in bun stuff, but happy to take a PR that makes @jest/globals as import source configurable.

A caveat is that there's no guarantee the semantics this plugin assumes holds when using bun instead (I've not used it at all, and I didn't know it lifted its test API from Jest). But yeah, making the import name configurable seems reasonable to me regardless 🙂

@TkDodo
Copy link
Author

TkDodo commented Sep 14, 2023

what the bun test runner does is trying to be a drop-in replacement for jest & vitest; so even if you have an import from @jest/globals, if you run the test with bun, it will swap it out internally. Same for vitest.

but happy to take a PR that makes @jest/globals as import source configurable.

thank you, I'll try to take a stab it at. Am I assuming correctly that '@jest/globals' should be the default for that option, and that it should be an Array of strings? Or is a single string resource enough (I would guess it is ..)

@G-Rath
Copy link
Collaborator

G-Rath commented Sep 14, 2023

I'm not too thrilled about this because we're meant to be just for Jest but I guess supporting a setting for it wouldn't hurt though

even if you have an import from @jest/globals, if you run the test with bun, it will swap it out internally

Doesn't that mean you can just import from @jest/globals and everything will work? in my eyes that's better anyway because you're saying "I want the Jest API" - the fact that Bun provides its own Jest-API-compatible module is an implementation detail.

I'd also be interested in hearing from @JoshuaKGoldberg on if this would help with #1289 - my understanding is vitest is also meant to be a drop-in replacement for Jest so in theory this new setting should help but they're already reported that not all rules are 1:1 compatible

@TkDodo
Copy link
Author

TkDodo commented Sep 14, 2023

Doesn't that mean you can just import from @jest/globals and everything will work? in my eyes that's better anyway because you're saying "I want the Jest API" - the fact that Bun provides its own Jest-API-compatible module is an implementation detail.

Yes, you can, but then we'd need an additional dependency on @jest/globals and add the types as well. I don't really prefer the "interop mode". I also think that some rules are not really jest specific, like no-focused-tests is something I definitely need no matter the test framework to avoid accidentally committing a test I had focussed on during development.

the suggested eslint-plugin-tests sounds very interesting in that regard.

I'm not too thrilled about this because we're meant to be just for Jest but I guess supporting a setting for it wouldn't hurt though

I can totally live with the patch we have right now, so let's take the time to settle on the right solution before introducing a new api :)

@JoshuaKGoldberg
Copy link
Contributor

I'd also be interested in hearing from @JoshuaKGoldberg on if this would help with #1289

Yeah that'd be great 😄 but for the mentioned caveats that the various testing libraries are all at least a little different from each other. Maybe it'd be useful to have a big comparison table of all the differences & similarities? (not volunteering myself, sorry)

@SimenB
Copy link
Member

SimenB commented Sep 14, 2023

I have no interest in maintaining a comparison table (or changing default behaviour). But an option to allow people to mix and match seems fine as long as we state no compat promises are made

Copy link

github-actions bot commented Apr 6, 2024

🎉 This issue has been resolved in version 28.2.0 🎉

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
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants