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

fix(no-deprecated-functions): do not attempt to detect version of Jest automatically #896

Closed
wants to merge 2 commits into from

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Sep 14, 2021

BREAKING CHANGE: no-deprecated-functions will require a Jest version specified in shared settings

Resolves #874
Resolves #886
Resolves #888
Resolves #889

…t automatically

BREAKING CHANGE: `no-deprecated-functions` will require a Jest version
specified in shared settings
@SimenB SimenB requested a review from G-Rath September 14, 2021 10:58
@G-Rath
Copy link
Collaborator

G-Rath commented Sep 14, 2021

@SimenB I would really like to keep this functionality as I think it's really useful - keep in mind as well that this rule is in our recommended, so this change would mean that users would have to have an extra bit in their configs to use the recommended.

Likewise for that same reason, I think this rule is being used without issue by the majority of our consumers; I think instead we should just amend the documentation to include how to "automatically" get the jest version if you're trying to run the linting in a monorepo-like (but not actually since shared packages are not at the toplevel) setup.

Alternatively, we should be able to abstract the require.resolve call into its own file which we could then mock - I'd be happy shipping that if it allowed keeping this functionality.

@SimenB
Copy link
Member Author

SimenB commented Sep 14, 2021

@G-Rath if you feel strongly about this I won't argue 🙂 I just wanna simplify the logic and remove any "magic" that causes confusion or errors for users. But again, your view of the situation weighs heavier in this case

@G-Rath
Copy link
Collaborator

G-Rath commented Sep 17, 2021

@SimenB yeah that's understandable, and I agree with the principle but think we can get this sorted without needing to remove the feature.

For the short-term I'll close this and make a PR updating the readme to mention that you'll need to provide the jest version in the config using require('jest/package.json').version in some conditions, while I have a go at seeing if we can get it working for this other case automatically.

@G-Rath G-Rath closed this Sep 17, 2021
@G-Rath G-Rath deleted the no-get-jest-version branch October 3, 2021 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants