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

Are you open to support context as describe alias? #83

Closed
raulmt opened this issue Feb 16, 2018 · 12 comments · Fixed by #1129
Closed

Are you open to support context as describe alias? #83

raulmt opened this issue Feb 16, 2018 · 12 comments · Fixed by #1129

Comments

@raulmt
Copy link

raulmt commented Feb 16, 2018

Up until around 1 year ago, context was an officially supported describe alias, but later got removed. Still, because of its value, some people (myself included) still want to use it, and as explained on that issue it's very easy to add it back.

Unfortunately, it's not compatible with using this ESLint plugin (and I really like this plugin :) ). Would you be open to merge a PR adding context (and derived context.only, …) to the describe aliases and other places where it would be needed?

I understand maybe you wouldn't want to have defaults that are different to what jest itself has in terms of globals, but if so, maybe adding it as a configurable option instead? Afaik, the plugin itself cannot receive a configuration though, so apparently that would need to be done by passing options to each applicable rule.

If you are open to this I can give a PR a try. Or if you have any other suggestion on how to use ´context´ and this plugin at the same time, I would love to hear it.

Thanks for this plugin!

@SimenB
Copy link
Member

SimenB commented Feb 17, 2018

I'm open to a PR allowing aliases for the globals 🙂

Afaik, the plugin itself cannot receive a configuration

This is possible, see https://eslint.org/docs/user-guide/configuring#adding-shared-settings

@raulmt
Copy link
Author

raulmt commented Feb 19, 2018

Thanks @SimenB . I'l give the PR a try then :)

And good to know about the shared settings!

@leoneves
Copy link

@raulmt do you managed to config context as aliases to describe through Adding Shared Settings?
You can show me how do you done that?

thanks in advance.

@raulmt
Copy link
Author

raulmt commented Feb 21, 2019

@leoneves no, I haven't done this yet, sorry…

robertknight added a commit to preactjs/preact that referenced this issue Mar 31, 2020
The Preact tests generally don't use `context` blocks and the ESLint rules
don't know to treat a `context` block like `describe` (see
jest-community/eslint-plugin-jest#83)
@brewster1134
Copy link

brewster1134 commented Jul 11, 2021

👍 for this as well. i fell in love with context when using rspec. after adding the jest-plugin-context plugin, i now get several eslint Duplicate _____ in describe block errors

@brewster1134
Copy link

its always strange coming across your old comments... any plans on adding this support?

@G-Rath
Copy link
Collaborator

G-Rath commented Feb 3, 2022

@brewster1134 yes, this is something we would like to have and on the face it's not even that hard to do - the wrinkle is with how ESLint currently handles settings which makes it a hard API to work with: it does a weird "replace merge", instead of straight overriding or merging.

For example, if we had describeBlockNames with a default value of ['describe', 'fdescribe', 'xdescribe'] and you wanted to add context, providing just ['context'] would result in ['context', 'fdescribe', 'xdescribe'] - instead you'd have to do ['describe', 'fdescribe', 'xdescribe', 'context'].

I found this out when starting to write tests for an initial settings-based implementation, and it was very confusing so I would expect it to be even more confusing for developers trying to configure their linter.

Apparently the new "flat" config system ESLint is switching to does away with this, so currently I'm sort of just waiting until either:

  • I find more time to look into it (unlikely when there are higher priority things, as this is very much a "nice to have")
  • someone else gives it a go and opens a PR (which'd include good documentation explaining how to configure settings)
  • the new config system comes out (which'll give me something new to work with that'd make implementing hopefully a lot more straightforward)

@brewster1134
Copy link

after looking at the code, it does use a whitelist approach. adding support for context is an easy update, but it sounds like it's preferred to support it as a configuration rather than in the plugin itself?

@G-Rath
Copy link
Collaborator

G-Rath commented Feb 4, 2022

Adding support for assuming that there is a function provided by Jest called context in the same way there is describe, it, etc is straightforward yes; however it wouldn't be correct to do because Jest doesn't provide such a function as it was removed in jestjs/jest#2384.

What we're open to is supporting providing this plugin with a list of aliases for these Jest functions as part of it's configuration, for which context is a prime use-case of.

@G-Rath
Copy link
Collaborator

G-Rath commented May 14, 2022

Once I've landed the implementation for #1106 you'll be able to do this with @jest/globals, as this:

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

is valid and will be understood by eslint-plugin-jest to be describe.

@G-Rath
Copy link
Collaborator

G-Rath commented May 28, 2022

I've opened #1129 that implements this, but I'm on the fence about actually landing it given you should be able to archive the same result by just importing from @jest/globals (which is required for ESM anyway).

@github-actions
Copy link

github-actions bot commented Jun 4, 2022

🎉 This issue has been resolved in version 26.5.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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants