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

Simplifying configuration #7185

Open
SimenB opened this issue Oct 16, 2018 · 29 comments
Open

Simplifying configuration #7185

SimenB opened this issue Oct 16, 2018 · 29 comments

Comments

@SimenB
Copy link
Member

SimenB commented Oct 16, 2018

This is a bit rambling, sorry about that. Feel free to edit this to clean it up and add more to it.

Jest currently has 51(!!) configuration options, many of which overlaps or intentionally overrides other options. (CLI options are out of scope for this issue, but they are obviously very much related)

They fall into a few different categories:

  • file matching
    • some are arrays, some are not.
    • some are globs, some are regexes
      • collectCoverageFrom
      • coveragePathIgnorePatterns
      • forceCoverageMatch
      • modulePathIgnorePatterns
      • testMatch
      • testPathIgnorePatterns
      • testRegex
      • transformIgnorePatterns
      • unmockedModulePathPatterns
      • watchPathIgnorePatterns
  • modules and mocking
    • confusing overlap with node's require api (node paths, extensions)
      • resetMocks vs clearMocks vs restoreMocks
      • automock
      • moduleDirectories vs modulePaths (I honestly have no idea)
  • Coverage things
    • reporters
    • thresholds
    • output directory
  • Setup files
  • it keeps going (feel free to edit)

First of all, I'd like to simplify the file matching a lot. Both to make it easier to use, but also easier to implement and reason about.

For file matching I think coveragePatterns: Array<Glob>, testPatterns: Array<Glob>, transformPatterns: Array<Glob> and remove everything else. You can use negated patterns to exclude things instead of a ignore thing. No more force to override ignores.

We could also group things (using the current names, although they are subject to change):

  • coverage can have collectCoverage, coverageDirectory, coverageReporters, coverageThresholds, collectCoverageFrom
  • setup can have globalSetup, setupTestFrameworkScriptFile, setupFiles, globalTeardown.
    • @palmerj3 had a cool idea about having just a single setup which exported different functions. Not as declarative, but maybe better?
  • module can have automock, resolver, moduleDirectories, modulePaths, moduleNameMapper, moduleFileExtensions, resetModules
  • mocks can have resetMocks, clearMocks, restoreMocks, timers (automock?`)
  • snapshots can have snapshotSerializers, snapshotResolver
  • notify and notifyMode can be combined (true is default mode, string sets the mode)

Another thing that's somewhat confusing is the difference between ProjectConfig and GlobalConfig. While the separation makes sense, it's invisible to users, and hard for them to reason about. It also doesn't work well with presets.


Finally, I leave you with this awesome article https://fishshell.com/docs/current/design.html 🙂

Every configuration option in a program is a place where the program is too stupid to figure out for itself what the user really wants, and should be considered a failure of both the program and the programmer who implemented it.

@rubennorte
Copy link
Contributor

I completely agree we should rethink our configuration options, so thank you for collecting all this.

Some comments:

For file matching I think coveragePatterns: Array, testPatterns: Array, transformPatterns: Array and remove everything else. You can use negated patterns to exclude things instead of a ignore thing. No more force to override ignores.

I think we should favor regular expressions over globs, as they're more expressive and anything that can be expressed as a glob can be as a regexp. I'd also keep the ignore versions as they're simpler than negated patterns.

We could also group things (using the current names, although they are subject to change):

I'm not sure we should group configuration options in objects. I've usually found flat options easier to deal with and it aligns better with CLI options (e.g.: you could overwrite the collectCoverageFrom config option with a --collectCoverageFrom CLI option).

@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

I think we should favor regular expressions over globs, as they're more expressive and anything that can be expressed as a glob can be as a regexp.

The syntax is also way harder, though. I find globs more readable, and easier to write (as you can probably do ls myglob in the terminal to test, that's hardet with regex). Escaping globs are easier as well, I think.

I'd also keep the ignore versions as they're simpler than negated patterns.

As long as we allow an array of globs and apply them in order, it's easy enough to just include an !. Harder with regexes, though.

I'm not sure we should group configuration options in objects. I've usually found flat options easier to deal with and it aligns better with CLI options

I agree on this one, at least depending on how yargs could handle deep objects. If you can do coverage.pattern=blah that's just as easy/readable imo

@jamietre
Copy link
Contributor

The syntax is also way harder, though. I find globs more readable, and easier to write (as you can probably do ls myglob in the terminal to test, that's hardet with regex). Escaping globs are easier as well, I think.

Also a fan of globs for file pattern matching. As long as you can pass arrays for all matchers, the limitations vs. regexp seem like not a big deal.

Another alternative (or addition) that would be great is just allowing any file pattern config to also accept (file: string) => boolean as its argument and let the consumer do whatever they want.

@thymikee
Copy link
Collaborator

Glob arrays + functions seems like a nice compromise of ease-of-use vs power to me.

@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

Passing a function could definitely work. Would it be inline, or point to some file that exports a function? Both?

Edit: might be confusing that a string can be interpreted as a module, so I guess inline is the way to go

@rubennorte
Copy link
Contributor

Passing a function could definitely work. Would it be inline, or point to some file the exports a function? Both?

It should be a function because otherwise it'd have to be split in two different configuration options (we can't distinguish a string being a glob or a file containing the function).

@jamietre
Copy link
Contributor

jamietre commented Oct 18, 2018

It should be a function because otherwise it'd have to be split in two different configuration options (we can't distinguish a string being a glob or a file containing the function).

Yes, this seem the most flexible since anyone is always free to just import their function.

Another related discussion here is that a some other config options require that something that's really just a function be implemented as an npm package, e.g. resolver testResultsProcessor, etc.

I'd prefer just allowing config to accept a function for these; people can always implement it as a module themselves if they want to. But it's a lot more cruft to make a module, link it in package.json, etc for these things when I'd rather just import it directly where it's consumed.

@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

It's something from when config had to be in package.json. I'm all for saying "use js config". Presets should help keep boilerplate down and if that's an issue for people.

We also have to consider people passing stuff from the CLI, but I don't think that too important for stuff that rarely change (like result processors, resolver, etc).
The use case of setting certain reporters etc on CI only is easily solved in js config by inspecting the env (or using is-ci)

@jamietre
Copy link
Contributor

I suppose you could overload again, and accept a string or an fn, where a string is always treated as an npm package? Specifying a reporter by npm package name on the CLI could be useful, though interestingly that can't be done now afaict ;)

@SimenB
Copy link
Member Author

SimenB commented Oct 20, 2018

You can do jest --reporters jest-junit today

@kentcdodds
Copy link
Contributor

This is absolutely stupendous. Thank you all for your work here! It's long been a point of confusion for me.

@oleersoy
Copy link

Resuggesting the above linked issue here:

Ideally it would be very simple to configure Jest to use the same glob pattern that typescript uses to resolve paths.

Initially I assumed it had that type of support, but discovered it's regex only per this so question.

If this is supported users can use the same glob pattern in jest that they would use in typescript.

I don't know about the rest of you, but I break out into a cold sweat any time RegEx is mentioned.

@SimenB
Copy link
Member Author

SimenB commented Jan 24, 2019

As part of this, I'd also like to use cosmiconfig to load configuration from files. It'll both allow us to delete code to look for a configuration file, and it'll allow us to support a few more ways to provide config (we currently have jest.config.js, .jestrc and jest in packages.json - all of which are supported by cosmiconfig)

@Farwatch
Copy link

Farwatch commented Feb 1, 2019

Reposting here at request of @SimenB : #7757

TL;DR: jest.restoreAllMocks() -> jest.restoreAllSpies(), and in config restoreMocks -> restoreSpies

@jeysal
Copy link
Contributor

jeysal commented Feb 10, 2019

Would be nice to provide a way to specify reporter options (and possibly other things) from the CLI. See #7845

Edit: Sorry, out of scope 😄

@SimenB
Copy link
Member Author

SimenB commented Feb 10, 2019

From OP:

CLI options are out of scope for this issue, but they are obviously very much related

But yeah, we should figure out a plan. I think as long as we really standardize on how to pass options (I like the reporter:s [name, [name, options]] pattern) it should be possible to figure out some relatively ergonomic way of doing it

@dandv
Copy link
Contributor

dandv commented May 1, 2019

On globs vs. regexp: I've been repeatedly confused by the patterns in test* options, and so have others. Turned out we didn't know about micromatch. One way to eliminate the confusion would be to have options names specifically with a RegExp or Glob suffix.

On simplifying configurations: any thoughts on cascading/extending/inheriting hierarchical config files? Here's a StackOverflow post requesting this feature.

I think ESLint does a great job at this with cascading configurations. It's been very easy to use in monorepos: have an .eslintrc in the monorepo root with rules common to all projects, then in individual projects that need something different (e.g. env: { browser: false, node: true } vs. the other way around), only add those rules to a project-level .eslintrc. I've ❤️ed this setup for a long while.

@icopp
Copy link

icopp commented May 6, 2019

@dandv That also roughly echoes what Babel is moving towards, with one root babel.config.js and then .babelrc in folders that need configuration overrides.

@jeysal
Copy link
Contributor

jeysal commented May 23, 2019

Some random thoughts from a quick chat with @SimenB:

  • Should users require.resolve more things themselves? Would solve issues like overriding babel-jest version without making it a peer dependency that has to be installed explicitly. Not good for JSON config, but perhaps users who use more complex config are likely to use JS config anyway.
    • Not so serious side note: For CLI usage, just jest --transform $(node -p 'require.resolve("babel-jest")') 😝
  • Is <rootDir> a good thing in JS config where you already have path.resolve etc like you use for e.g. webpack config? Probably wouldn't implement it today, instead just telling people to resolve it themselves, but we already have it and it works fine so idk. Plus it's still quite a bit shorter to write.
  • Would be great to somehow find out what percentage of users use which configuration method (at least on public gh or something), but I haven't dug that deep into the advanced search queries yet 😄

@tiendq
Copy link
Contributor

tiendq commented Nov 12, 2019

Totally agree, configuration should be simpler and it saves us from wasting time to decide right option to use (while they're same :D) e.g. testRegex vs. testMatch in my last code review

@thernstig
Copy link
Contributor

Is there any reason to having both setupFiles and setupFilesAfterEnv? I cannot see any downside of only having setupFilesAfterEnv?

@jeysal jeysal unpinned this issue Dec 11, 2019
@thernstig
Copy link
Contributor

thernstig commented Dec 16, 2019

Is there any reason to having both setupFiles and setupFilesAfterEnv? I cannot see any downside of only having setupFilesAfterEnv?

I made a separate issue for this: #9314

@MynockSpit
Copy link

MynockSpit commented Jan 29, 2020

Curious, b/c I don't see it here (maybe I'm blind?) but does this include simplifying/clarifying the number of ways to make a mock or mock-like thing (automock, spyOn, jest.mock, __mocks__, requireMock, jest.fn)?

@SimenB
Copy link
Member Author

SimenB commented Jan 29, 2020

Hey there! No, this is for https://jestjs.io/docs/en/configuration and https://jestjs.io/docs/en/cli, not anything else. The features you mention are more documentation issues, I think.

@villesau
Copy link

villesau commented Feb 1, 2021

I think the most confusing overlapping configuration options are the different setup options:

  • setupFiles
  • setupFilesAfterEnv
  • globalSetup
  • testEnvironment
  • And all the after -versions of above

All of these are a bit different flavour of essentially the same thing. What makes it especially difficult is that there's no clear explanation which is used for what purposes and when they are called, are they async or not and so on. And how they differ from each others. To me it looks that all of them have came when someone has needed a new place to setup something and was too afraid to change the existing logic.

@SimenB
Copy link
Member Author

SimenB commented Feb 24, 2022

I still want to do this at some point, but I'm not sure when I'll have the energy to take it on.

Current plans for Jest and breaking changes are: V28 is right around the corner and will be filled with breaking changes., mostly minor or easy to tweak your code for. V29 will most probably only change snapshotFormat defaults and drop node 17 as its only breaking changes, to make the upgrade as smooth as possible (beyond having to regenerate all snapshots (or change config to current options)). But maybe Jest v30 could be the big config overhaul? A boy can dream 😀

Anyways - my current thinking before I postpone my reminder yet again: My current thinking hasn't changed much from the OP for file matching (i.e. I'd like to go for just arrays of globs), but we can probably support a function in addition (note that since we run in workers, it would have to be a string pointing to a module exporting a function). I'm thinking they'd be mutually exclusive - either provide globs or a custom function. We tested out doing just globs when I visited FB a few years ago, and it didn't cover their use cases. So a function seems reasonable.


@villesau (you might know, but others reading might not)

setupFiles before the test framework is loaded - typically where you do polyfilling
setupFilesAfterEnv after the test framework is loaded - typically where you add custom matchers, setup spies/stubs etc
globalSetup runs once per project instead of once per test suite (file) - typically where you start a browser or db (and close it in globalTeardown)
testEnvironment the thing that sandboxes and runs the code in tests

They all serve different purposes and cannot really be interchanged (except most setupFiles could be setupFilesAfterEnv, I guess). testEnvironment is for more advanced use cases (like exposing puppeteer instance or a db connection to tests, working around the sandbox) that most users won't need.

That said, we should probably document these better, and explain when to use which.

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

No branches or pull requests