Allow config.transform[regex] to be a function for jest.runCLI(config)

[jharris4]

Currently with jest.runCLI you can only pass JSON parseable strings like this:

const jest = require('jest);

jest.runCLI(
  {
    config: '{ "testMatch": "<rootDir>/test/*_spec.js", "transform": { "^.+\\.jsx?$": "require-path-to-transform" } }'
  }
);

If we want to create a utility function to run jest with a custom configuration like:

const jest = require('jest);

function runJest(someOptions) {
  jest.runCLI({ config: createConfigFromOptions(someOptions) });
}

We run into the problem that createConfigFromOptions can only configure the preprocessor transforms by specifying path to files, leading to the need to split the configuration across multiple source files.

With this PR that workflow is much nicer in that you can just do:

const jest = require('jest');

jest.runCLI(
  {
    config: {
      testMatch: [
        "<rootDir>/**/*_spec.js"
      ],
      transform: {
        "^.+\\.jsx?$": () => ({
          process: (src, filename) => src + 'you can transform the file src right here'
        })
      }
    }
  }
);

No extra source files required for the transform to work! :-)

Several tests were added to show that it works properly, including a new test that actually calls jest.runCLI from within a test!

[codecov-io]

Codecov Report

Merging #7398 into master will increase coverage by 0.87%.
The diff coverage is 64.7%.

@@            Coverage Diff             @@
##           master    #7398      +/-   ##
==========================================
+ Coverage   66.79%   67.66%   +0.87%     
==========================================
  Files         242      242              
  Lines        9371     9374       +3     
  Branches        5        5              
==========================================
+ Hits         6259     6343      +84     
+ Misses       3110     3029      -81     
  Partials        2        2

Impacted Files	Coverage Δ
packages/jest-config/src/index.js	18.75% <0%> (-0.61%)	⬇️
packages/jest-config/src/normalize.js	85.53% <100%> (ø)	⬆️
packages/jest-runtime/src/script_transformer.js	88.67% <100%> (+0.21%)	⬆️
...es/jest-cli/src/lib/handle_deprecation_warnings.js	0% <0%> (ø)	⬆️
packages/jest-cli/src/reporters/utils.js	91.34% <0%> (+0.96%)	⬆️
packages/jest-cli/src/TestScheduler.js	74.19% <0%> (+8.87%)	⬆️
packages/jest-cli/src/SearchSource.js	70.51% <0%> (+12.82%)	⬆️
packages/jest-cli/src/TestWatcher.js	57.14% <0%> (+14.28%)	⬆️
packages/jest-cli/src/ReporterDispatcher.js	100% <0%> (+25%)	⬆️
...ackages/jest-cli/src/reporters/verbose_reporter.js	79.62% <0%> (+59.25%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3c10cb...e48fd77. Read the comment docs.

[jharris4]

@rickhanlonii regarding your comment #7288 (comment)

The main benefit that comes to my mind is specifying the options via cli, which is v common particularly in CI environments

I agree that supporting JSON/String only configurations for use by the cli is a very important use case.

That's why this PR was implemented as a non breaking change with full backwards compatibility for that use case.

In fact, prior to this PR passing an object instead of String to jestCli.run threw an obscure exception which this PR now improves by throwing a more explicit error.

There were also no tests of the jestCli.run or jestCli.runCLI functions and this PR adds test coverage for those, and shows that the existing String only cli configuration still works as expected.

If you're interested those new tests can be found here:

https://github.com/facebook/jest/blob/e48fd77c55c9cff64eb349e6a367ab3d897a8c15/packages/jest-cli/src/__tests__/cli/run.test.js

https://github.com/facebook/jest/blob/e48fd77c55c9cff64eb349e6a367ab3d897a8c15/packages/jest-cli/src/__tests__/cli/runCLI.test.js

[SimenB]

Suggested change

	return Promise.resolve(results);
	return results;

[SimenB]

I don't think we can do this - the config must be json serializable since we pass it to workers

[jharris4]

Hi Simeon thanks for the review! It looks like Rick Hanlon has moved on to other projects and I wonder if anyone at Facebook will keep maintaining Jest??

Can you please elaborate on such a sweeping statement about workers? Ex: How are workers used by jest and why does this require JSON?

And even if that holds true, is it worth restricting how Jest can be used? Not having jest be runnable except via CLI actually makes it unusable for my use cases.

We shouldn’t limit how it can be used with out good reason. It’s written in JavaScript so it’s be nice if it could be executed using JavaScript. JSON is very limiting! 😉

[yordis]

Hi Simeon thanks for the review! It looks like Rick Hanlon has moved on to other projects and I wonder if anyone at Facebook will keep maintaining Jest??

It is open source ❤️ we can keep maintain the project without relying on Rick Hanlon 👏

Can you please elaborate on such a sweeping statement about workers? Ex: How are workers used by jest and why does this require JSON?

@jharris4 this is where I stopped 😄 It does require JSON because it can't serialize JavaScript prototypes. Read #7279 (comment)

Now that I understand, it is a limitation.

We shouldn’t limit how it can be used without good reason. It’s written in JavaScript so it’s be nice if it could be executed using JavaScript. JSON is very limiting! 😉

Trade off, fix NodeJS itself, that will be amazing 👏

I believe that #7288 is a better way to go.

Yes, JSON is limited, but no, you can do many things by passing data around, rarely you will need an actual JavaScript object to be passed around for these cases, I am curious to know your use cases so we can figure out how to properly implement the feature that will fit your requirements.

[SimenB]

Can you please elaborate on such a sweeping statement about workers? Ex: How are workers used by jest and why does this require JSON?

Workers are how jest parallelizes execution across cpu cores. And in order to talk between different processes, you need to serialize the data. This is how process.send works in node: https://nodejs.org/api/process.html#process_process_send_message_sendhandle_options_callback

So while we're not technically bound to the config being JSON-serializable, we're bound to it being serializable to a string, which functions are not.

EDIT: You can do function.toString() to get the implementation of a function, but you won't get things that are in closures, so it's pretty much just as limiting and probably more confusing to the end user

And even if that holds true, is it worth restricting how Jest can be used? Not having jest be runnable except via CLI actually makes it unusable for my use cases.
We shouldn’t limit how it can be used with out good reason. It’s written in JavaScript so it’s be nice if it could be executed using JavaScript. JSON is very limiting!

We definitely want to improve the programmatic usage, but passing in functions in configuration is not something we can do due to the technical limitations mentioned above.

One thing I've had on my todo block (pretty far down) for a long time is rip jest-cli apart so that it's just the CLI part, and create a new jest-api or something that both jest-cli and potentially other consumers could use to run Jest. At that point, API limitations should be more visible and something we could try to address.

[jharris4]

@yordis thanks for your input. My use case is that we've put together a build tool that has the config for babel/rollup/webpack/jest all in one file (I know sounds crazy but it's working nicely!). So it was bothering me that Jest essentially required a special separate file that then needs to separately parse the master config just to setup babel.

But I see now that it is indeed a limitation of the child_process internal module of node (I tried hacking on it a bit this morning lol)

@SimenB thanks for the explanation about the workers. I dug into the code a bit this morning, and you are indeed correct that config transform as functions do get swallowed (Silently!) by the send call to the child process forked by the jest-worker package.

I may play around a little bit more to see if I can get past this limitation somehow, but otherwise I guess this PR should be ignored (closed?) since it won't work with parallel test runs.

As for your jest-api vs jest-cli package split idea, I think that's a really good idea. I had been thinking about the same idea myself.

[yordis]

@yordis thanks for your input. My use case is that we've put together a build tool that has the config for babel/rollup/webpack/jest all in one file (I know sounds crazy but it's working nicely!).

Sounds like my CLI, we are doing the same with the same use cases pretty much.

Today you could serialize the values into an environment variable and deserialize later on your Jest config.

Here is an example.

https://github.com/neutrinojs/neutrino/blob/542604c3416ff2256eeb3dc4bed88093f17031dc/packages/jest/src/index.js#L25

https://github.com/neutrinojs/neutrino/blob/542604c3416ff2256eeb3dc4bed88093f17031dc/packages/jest/src/transformer.js#L3

There is no workaround for dealing with this serialization problem.

At the end of the day, you could put some factory function on your Jest transformer that takes the configuration and recreate the Babel config.

const { createTransformer } = require('babel-jest');
const { createConfig } = require('my-package/babelConfigFactory');

const options = JSON.parse(process.env.JEST_BABEL_OPTIONS);
const babelOptions = createConfig(options);
module.exports = createTransformer(babelOptions);

So would be rare that you have to pass a full JavaScript object between workers.

In the future, you will remove the hacky serialization using environment variables but will need to keep the factory function.

Just keep your configuration as simple as possible so the factory function can't depend on JavaScript unserializable objects.

Let me know if you need any help, if your tool is open source I can give you a hand or direction.

[jharris4]

Wow, neutrino looks super interesting! It indeed has many similar ideas to what we've been working on.

We've been building our libraries and web apps using an internal tool that we named porter. We're planning to open source it fully once we've extracted some proprietary bits, and a very preliminary version can be found here: https://github.com/artsman/porter

The idea is that any project/package using porter has one central porter.js configuration file, with a structure something like:

const porterBabelOptions = {
  targets: [
    "ie11", "last 2 versions"
  ],
  useJSX: true,
  useDecorators: true
};

const porterWebpackOptions = {
  input: 'src/app/index.js',
  outputPath: 'dist',
  publicPath: '/static/',
  bundleName: 'app-[name]',
  babel: porterBabelOptions,
  split: true,
  splitVendors: true,
  hmr: false,
  reactHot: false
};

const porterServerOptions = {
  port: 8181,
  host: 'localhost',
  proxy: {
    '/api/data', 'https://localhost:1234/api/data'
  },
  hotMiddleware: false
};

const porterConfig = {
  tools: {
    babel: getBabelConfig({
      input: 'src',
      ...porterBabelOptions
    }),
    rollup: getRollupConfig({
      input: 'src/index.js',
      output: 'dist/my-lib.js',
      babel: porterBabelOptions,
      licenseFile: 'LICENSE.txt'
    }),
    webpack: getWebpackConfig(porterWebpackOptions),
    webpackServe: {
      server: porterServerOptions,
      webpack: getWebpackConfig(porterWebpackOptions)
    },
    webpackDev: {
      server: {
        ...porterServerOptions,
        hotMiddleware: true
      },
      webpack: getWebpackConfig({
        ...porterWebpackOptions,
        hmr: true,
        reactHot: true
      })
    },
    jest: getJestConfig({
      inputPath: 'test/',
      testMatch: '*_spec.js',
      babel: porterBabelOptions
    })
  }
};

modules.exports = porterConfig;

Each of the functions: getBabelConfig, getRollupConfig, getWebpackConfig, getJestConfig are kind of like presets that use options to generate fully formed configurations for each of the tools.

These configs can then be executed by the respective tool using cli commands like: porter-babel, porter-rollup, porter-webpack etc...

The idea is that the getBabelConfig call above returns something like:

{
  presets: [
    ["@babel/preset-env", { targets: [ "ie11", "last 2 versions" ]}]
  ],
  plugins: [
    "@babel/plugin-react-jsx",
    "@babel/plugin-decorators"
  ]
}

And the getJestConfig call returns something like:

const babelCore = require('@babel/core');
const babelConfig = getBabelConfig(porterBabelOptions);

{
  testMatch: [
    "<rootDir>/test/*_spec.js"
  ],
  transform: {
    '^.+\\.js$': () => ({ process: (source) => babelCore.transform(source, babelConfig) })
  },
  
}

Basically I created this PR so that the getJestConfig function won't require indirection to an external file for the transform function. :-)

[SimenB]

I may play around a little bit more to see if I can get past this limitation somehow, but otherwise I guess this PR should be ignored (closed?) since it won't work with parallel test runs.

If you make it work, that would be awesome, but I think it's such a fundamental thing that's it's impossible to work around. Happy to be proven wrong, though!

Happy to reopen (and please continue the discussion! 🙂)

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.