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

New rule request: no-hooks #72

Closed
macklinu opened this issue Feb 11, 2018 · 3 comments · Fixed by severest/retrobot#29
Closed

New rule request: no-hooks #72

macklinu opened this issue Feb 11, 2018 · 3 comments · Fixed by severest/retrobot#29

Comments

@macklinu
Copy link
Collaborator

eslint-plugin-mocha has a rule called no-hooks that I think could be a good addition to this ESLint plugin. I think the mocha/no-hooks docs as well as this tweet help explain how using nested describe() and beforeEach() hook functions can result in harder to follow code / shared state between tests.

I think when combined with jest/consistent-test-it, you could enforce tests that look more like:

function setupFoo(options) { /* ... */ }
function setupBar(options) { /* ... */ }
function setupBaz(options) { /* ... */ }

test('foo does this', () => {
  const foo = setupFoo({ /* options */ })
  expect(foo).toEqual(/* expected */)
})

test('bar does this', () => {
  const bar = setupBar({ /* options */ })
  expect(bar).toEqual(/* expected */)
})

test('baz does this', () => {
  const baz = setupBaz({ /* options */ })
  expect(baz).toEqual(/* expected */)
})

instead of looking like this:

let foo
let bar
let baz

beforeEach(() => {
  foo = setupFoo()
  bar = setupBar()
  baz = setupBaz()
})

afterEach(() => {
  foo = null
  bar = null
  baz = null
})

test('foo does this', () => {
  expect(foo).toEqual(/* expected */)
})

test('bar does this', () => {
  expect(bar).toEqual(/* expected */)
})

test('baz does this', () => {
  expect(baz).toEqual(/* expected */)
})

I think there are times when using setup and teardown hooks are necessary, but I find that they can often be avoided. I could see this rule providing an option that would whitelist allowed hook function names, or the user could disable with // eslint-disable-next-line jest/no-hooks.

Thoughts on this rule? Would you accept a PR for this feature?

@SimenB
Copy link
Member

SimenB commented Feb 12, 2018

Yeah, this sounds like a reasonable rule! PR most welcome 🙂

@thymikee
Copy link
Member

@macklinu
Copy link
Collaborator Author

I will look to contribute a PR for this today. 🌴

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants