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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create require-hook rule #929

Merged
merged 4 commits into from Oct 10, 2021
Merged

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Oct 10, 2021

tbh this was a bit of a no-brainer to have as a rule - the more I think about it and check my existing tests, the more I think "yeah there's no reason that can't/shouldn't be wrapped in a before hook" 馃槄

The docs should explain the important stuff - namely, that this rule flags pretty much all common "action" statements like re-assignments and function calls that are not within a hook.

I have excluded function calls on the jest global, like jest.mock but suspect some of those should actually also always be within a hook function?

I also think we should probably make this a recommended rule, but understand if you don't want to delay the new major any further.

Closes #752

@G-Rath G-Rath requested a review from SimenB October 10, 2021 02:36
@G-Rath G-Rath force-pushed the create-no-unhooked-function-calls branch from 175f0a4 to bbb173c Compare October 10, 2021 02:36
@G-Rath
Copy link
Collaborator Author

G-Rath commented Oct 10, 2021

I'm wondering if this rule should have a strict or loose option (depending on what we want as the default) to adjust how it handles cases like:

const myValue = getValue();

describe('some tests', () => {
  ...
});

For the above you could argue that myValue should be a let that gets assigned in a beforeEach, based on it being assigned the return from a function call (vs if it was a primitive literal, in which case it is fine).

But I wouldn't be surprised if some folks found it too noisy, due to having builder functions that are returning static values? 馃

@SimenB SimenB changed the title feat: create `require-hook rule feat: create require-hook rule Oct 10, 2021
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

@SimenB SimenB merged commit 6204b31 into main Oct 10, 2021
@SimenB SimenB deleted the create-no-unhooked-function-calls branch October 10, 2021 09:08
github-actions bot pushed a commit that referenced this pull request Oct 10, 2021
# [24.7.0](v24.6.0...v24.7.0) (2021-10-10)

### Features

* create `require-hook` rule ([#929](#929)) ([6204b31](6204b31))
* deprecate `prefer-to-be-null` rule ([4db9161](4db9161))
* deprecate `prefer-to-be-undefined` rule ([fa08f09](fa08f09))
@github-actions
Copy link

馃帀 This PR is included in version 24.7.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

github-actions bot pushed a commit that referenced this pull request Oct 10, 2021
# [25.0.0-next.6](v25.0.0-next.5...v25.0.0-next.6) (2021-10-10)

### Bug Fixes

* **lowercase-name:** consider skip and only prefixes for ignores ([#923](#923)) ([8716c24](8716c24))
* **prefer-to-be:** don't consider RegExp literals as `toBe`-able ([#922](#922)) ([99b6d42](99b6d42))

### Features

* create `require-hook` rule ([#929](#929)) ([6204b31](6204b31))
* deprecate `prefer-to-be-null` rule ([4db9161](4db9161))
* deprecate `prefer-to-be-undefined` rule ([fa08f09](fa08f09))
* **valid-expect-in-promise:** re-implement rule ([#916](#916)) ([7a49c58](7a49c58))
@github-actions
Copy link

馃帀 This PR is included in version 25.0.0-next.6 馃帀

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 this pull request may close these issues.

[new rule] Prevent calling functions in tests outside of it & before/after blocks
2 participants