feat(no-export): new rule for no-export

[benmonro]

I ran into this issue when we had a set of helper functions in 1 test file that
got exported directly from a -test.js file. The test that was importing this
file then executes both it's own tests as well as the tests in the imported
file. We started seeing phantom snapshots show up that we weren't able to find
the origin of until we finally tracked it down and realized that a test file was
exporting shared functions. We fixed this by moving those functions out into
seperate files, but that was the inspiration of this rule.

This rule aims to eliminate duplicate runs of tests by exporting things from
test files. If you import from a test file, then all the tests in that file will
be run in each imported instance, so bottom line, don't export from a test, but
instead move helper functions into a seperate file when they need to be shared
across tests.

[SimenB]

This is an awesome rule, thanks for working on it! 🎉

I'm a bit worried about it forcing consumers to only apply it to test files, as it'll report false positives on production code. While not a huge problem, and it keeps the complexity of the rule implementation down, would it be a lot of work to make sure there's actually tests in the file? Might not be worth it, but if so I think it should be noted in its docs :)

(I've been thinking about adding some way of accessing jest config in rules, then we could check if the test would be run as such by Jest. Nothing concrete, though)

[benmonro]

@SimenB actually not hard at all to check for a test and a great idea! I'll add that

[SimenB]

Not recommended (changing recommended rules is a breaking change)

[benmonro]

what's the process of getting it to be a recommended change? This rule prevents very hard to track down problems....

[SimenB]

Just remembering when we release a new major :)

[benmonro]

i'll remove it for now, but I would like to move this to recommended in another PR, if you don't mind providing guidance on that process.

[SimenB]

Sure! Just open up a PR updating the readme and add it to the config exported from index and I'll make sure to merge it before making a major release

[chrisblossom]

Sometimes you need to add an empty export {} in a typescript file to workaround getting typescript error error TS1208: Cannot compile namespaces when the '--isolatedModules' flag is provided microsoft/TypeScript#15230

Because of that, I think it would be helpful to let .test.ts or .test.tsx files have an empty export function.

[SimenB]

Because of that, I think it would be helpful to let .test.ts or .test.tsx files have an empty export function.

I'm down with an option (allowEmpty, or something), but I don't think it should be the default behaviour. People can use overrides in their config if they want 🙂

[benmonro]

can you merge @SimenB ? or give me permission to merge? :)

[benmonro]

I can do the allowEmpty option in another PR

[benmonro]

@SimenB any guidance you can provide on what it takes to get a rule to be 'recommended'? The bugs that come about are quite nefarious and time consuming to track down, especially for junior devs...

[benmonro]

@SimenB looks like the build failed on master. :( https://travis-ci.org/jest-community/eslint-plugin-jest/jobs/560608618

[SimenB]

🎉 This PR is included in version 22.11.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[SimenB]

@SimenB any guidance you can provide on what it takes to get a rule to be 'recommended'?

Not much guidance needed, just add the badge to the readme and add the rule here:

eslint-plugin-jest/src/index.js

Lines 43 to 55 in b3a360d

	rules: {
	'jest/no-alias-methods': 'warn',
	'jest/no-disabled-tests': 'warn',
	'jest/no-focused-tests': 'error',
	'jest/no-identical-title': 'error',
	'jest/no-jest-import': 'error',
	// 'jest/no-mocks-import': 'error',
	'jest/no-jasmine-globals': 'warn',
	'jest/no-test-prefixes': 'error',
	'jest/valid-describe': 'error',
	'jest/valid-expect': 'error',
	'jest/valid-expect-in-promise': 'error',
	},

I'll make sure to merge the PR for the major 🙂