Bumped jest-config version to fix vulnerabilities

[rossknudsen]

Lots of vulnerabilities observed using yarn audit. Bumping the version of jest-config seems to fix the issues.

[stephtr]

Thank you for the PR!
Apparently updating jest-config pulls in an updated version of cssstyle, which isn't compatible with our node version (error [email protected]: The engine "node" is incompatible with this module. Expected version ">=8".).
@connectdotz: As a mid-term fix, would it make sense to update the node version of the travis config?

[SimenB]

Oh, really? That's not supposed to happen - jest-config@24, like the rest of jest@24, supports node 6. Somebody in the dependency tree did not follow semver...

EDIT: jsdom/cssstyle#95 (comment)

You can probably do some lockfile trickery to keep a version of cssstyle that supports node 6 around

The semver range didn't change, so just reverting the changes to cssstyle in the lockfile should fix CI

[stephtr]

Since (according to jest's changelog) [email protected] only introduced one fix, are there any objections against merging?

[connectdotz]

hmm... remember we were talking about removing Settings.js and jest-config dependency because it is not used in this repo nor vscode-jest... maybe instead of bump the version, we could remove them...

[rossknudsen]

Lol. I'm trying fix an issue in vscode-jest-test-adapter that relies on Settings.js (see https:/rtbenfield/vscode-jest-test-adapter/blob/master/src/TestLoader.ts#L111).

It depends on whether you'd consider this functionality a core part of the API that you are providing as a support library to editor plugins. I'm happy if you want to remove, I'll just copy the code into that repository (or find another way to solve the problem).

[connectdotz]

I see... wasn't aware there is any library depends on Settings.js, thanks for the info. In that case, we should continue to support it.

@stephtr I have no objection with the merge.

[stephtr]

What about removing the default values of testMatch and testRegex and requiring to call getConfig instead? That way the two settings would always match the ones being used in reality and additionally we could drop the jest-config dependence.

@rossknudsen Would that be fine for you?

[rossknudsen]

@stephtr that sounds like a really good idea. The default values are probably not what I want and the actual values configured are.

What would happen if the user didn't configure either testMatch or testRegex implicitly leaving them with their default values? It looks like the getConfig method would only parse out the explicitly configured values, but I might be wrong.

[SimenB]

@rossknudsen it should work since it does --showConfig which will launch jest, load the config, and print it.

However (while clever) that's pretty hacky. Feel free to open up an issue with Jest asking for a loadConfig method. Second time recently it's been needed/requested and should remove the need for the getConfig method here (albeit it'd keep the dependency on jest-config)

[rossknudsen]

I just ran jest --show-config with neither testMatch nor testRegex present and got the following output:

{
// ...
  "testMatch": [
    "**/__tests__/**/*.[jt]s?(x)",
    "**/?(*.)+(spec|test).[tj]s?(x)"
  ],
  "testRegex": [],
// ...
}

Which should be reasonable and when I specify either value, the other one is defaulted to an empty array.

However, at the moment, the callback passed to getConfig is not being called and the settings are not being updated in the client code (vscode-jest-test-adapter codebase).

[stephtr]

albeit it'd keep the dependency on jest-config

I would be in favor of dropping. We only import it for two default values of jest, which often don't represent the actual environment. All for the price of significantly increasing the amount of (unnecessary) dependencies.

@rossknudsen: I just tested it, the following sample works for me (in an empty folder where jest and jest-editor-support had been installed):

const { Settings, ProjectWorkspace } = require('jest-editor-support');

const workspace = new ProjectWorkspace('.', 'node_modules/.bin/jest.cmd');
const settings = new Settings(workspace);

settings.getConfig(() => {
    console.log(settings.settings.testMatch);
});

[SimenB]

I meant you'd do await jestConfig.loadConfig() instead of spawning jest just to have it resolve config. Up to you if you want to keep your current approach, of course :)

[rossknudsen]

@stephtr thanks heaps for the code sample. It does seem to work mostly for me too. Except if I create a jest.config.js file and specify testMatch:

// jest.config.js

module.exports = {
  testMatch: [
    "**/__tests__/**/*.[jt]s?(x)",
    "**/?(*.)+(spec|test).[tj]s?(x)",
    "**/bogus.js"
  ]
};

Running on the command line displays the extra entry I added to the array but when I run the code sample, I only get the default config. Even if I add the pathToConfig to the ProjectWorkspace constructor, it doesn't seem to work. Here is what I have:

const { Settings, ProjectWorkspace } = require("jest-editor-support");

const workspace = new ProjectWorkspace(
  ".",
  "node_modules/.bin/jest.cmd",
  "./jest.config.js"
);
const settings = new Settings(workspace);

settings.getConfig(() => {
  console.log(settings.settings.testMatch);
});

Output: [ '**/__tests__/**/*.js?(x)', '**/?(*.)+(spec|test).js?(x)' ]

[stephtr]

Oh sorry, I was blinded by the output. On Windows you have to use backslashes for the path, with them instead of the slashes it is working correctly.

[rossknudsen]

Nice, thanks for that. Would be good if both path separators were handled, maybe a future PR.

I think I've solved the issues I had and based on the discussion here and in #14, I think I'll close this in favor of that PR.