Setup build as a target dependency for test and use compiled assets in tests

[yharaskrik]

Description

With the new version of Nx we can now define build for dependencies as a target that needs to be run prior to the original build command can be run using the targetDependencies in nx.json. It would be awesome to be able to use the compiled assets when running jests tests, rather than using jest-preset-angular with the *.ts transformer on a per spec file.

Essentially we would be able to put this in our nx.json

"targetDependencies": {
        "build": [
            {
                "target": "build",
                "projects": "dependencies"
            }
        ],
        "test": [
           {
               "target": "build",
               "projects": "dependencies"
            }
        ]
    }

and then when a spec file imports from TS such as @scope/lib1 instead of pointing that alias to /libs/lib1/src/index.ts we would point it to /dist/libs/lib1, this is similar to how the Nrwl builder will work --with-deps

Motivation

Should greatly speed up test runs if TypeScript files do not need to be compiled on the fly and takes advantage of the buildable deps Nx already provides.

Suggested Implementation

@nrwl/jest would need to look at the nx.json in the same way that the builders do and determine if there are targetDependencies and then when the tests are run, the jest.config.js that is used will need to be modified such that we can override the TS paths. Not exactly how this should work since I know jest-preset-angular is used to do the transforming with ts-jest underneath, maybe we can pass in a temporary tsconfig file to those libraries so that we can merge the root tsconfig.base.json with the paths that were build.

ts-jest will look at the moduleNameMapper (https://kulshekhar.github.io/ts-jest/docs/getting-started/paths-mapping/) for path matching, jest supports passing in moduleNameMapper (althought the typings expect a string in @types/jest for runCLI so not sure how that would work) which should allow us to override where sources are mapped too (I am assuming that those would override paths in the tsconfig but I am not 100% sure on that)

Alternate Implementations

[yharaskrik]

I would also be more than happy to help work on this if it is something Nrwl is interested in including.

[yharaskrik]

Here is a PoC: https://github.com/yharaskrik/jest-nx-build-dependencies

Issues so far:

1. By default (based on what is set in tsconfig.base.json) code is compiled into esnext by ng-packager:lite but Jest cannot parse that by default. If we change that libraries output to commonjs then it works and the test uses the compiled artifacts.

Solutions: Either have ng-packagr:lite emit the commonjs alongside the normal module value (if not already commonjs) OR setup Jest such that it can read ESM modules, although this is still experimental (https://jestjs.io/docs/ecmascript-modules) or set allowJs to true in the tsconfig so that the built artifacts in /dist are compiled down to the right module format using jest-preset-angular since the default transform pattern is: ^.+\\.(ts|js|html)$

There is a performance trade off with this because we either need to compile the code to commonjs right away (during the nx build <lib> step) or have the !commonjs code compiled to commonjs using the transform stage of the test run. The latter of which sort of negates some of why we would do this in the first place.

2. I read through the Jest source code and it seems that the moduleNameMapper: string property that can be passed into the runCLI function of Jest as a Config.argv property is not actually used so we cannot override the paths that way. This means that we would need to create a temp jest.config.js that takes the jest.config.js of the project being tested and adds the moduleNameMapper property with a Record<string, string | string[]> of paths to override based on the projects dependencies. The issue here is that we would need to traverse the AST of the jest.config.js file to do that and I don't currently see a package installed to do that (unless I am missing something there). I looked at how the nx move command works and it just updates the jest.config.js using RegEx, this is definitely possible to do but it adds a bit more iffy-ness as we never know what users do to their jest.config.js files in their libs.

Solutions: Either install an AST tranversal lib and use that to see if there is already a moduleNameMapper property and if so merge it or add one beside a key we always expect to be there (name?) or write some Regex to insert the moduleNameMapper in a place we always expect it to be (again maybe name?)

#2 is not that big of a deal, #1 is where the decision will come in, I would think it would be more optimal to pre-process the lib and compile commonjs assets as well since then those can be reused between all tests, rather than jest-preset-angular (or just ts-jest in the case of non angular projects) having to compile the esnext ESM artifacts down to commonjs on the fly.

Would love some feedback on this.

[yharaskrik]

So in regards to #1 above, ng-packagr-lite only compiles to esm2015 but if we were the normal @angular-devkit/build-angular:ng-packagr we also get a umd module, which does work when using in moduleNameMapper.

I will take a look at Nrwl's implementation of ng-packagr-lite and maybe we can conditionally enable umd modules if a flag is set to use compiled artifacts for tests instead of compiling on the fly.

[yharaskrik]

I now have a branch off of nx master that can conditionally compile dependencies to umd during the build process (https://github.com/yharaskrik/nx/tree/jaybell/allow-compiling-deps-to-umd-for-testing). Things to do now:

1. Need to be able to assign build as a target dependency to test (it currently infinitely loops and you get a max call stack error)
2. Decide where a flag should be added such that users can enable testingFromSource so that builds can read that and know if they need to compile umd or not
3. test target needs to check flag from 2 and use Nx utils to determine if all deps are built, if they are build the moduleNameMapper property of all dependencies that were built and add to the jest.config.js for the project
4. read in the jest.config.js for the project and add (or merge if it exists) moduleNameMapper from 3 and then write that to a temporary location
5. Call the runCLI command with the temporary jest.config.js instead of regular one
6. Disable testing from source if watch mode is true

[yharaskrik]

Confirmed we can run jest on a project using a config in a tmp directory.

yharaskrik/jest-nx-build-dependencies@2715600

[yharaskrik]

This PR has the functionality for the jest executor to determine if all of the deps have been built and if so create all of the moduleNameMapper values, creates a tmp jest.config.js and then uses that config file as the config for running the tests themselves, this will allow jest to use the compiled artifacts rather than compiling on the fly.

https://github.com/yharaskrik/nx/pull/2/files

[gms1]

after converting our workspace to nx, our unit tests are running about 4 times as slow as before
I found @yharaskrik's PR very interesting, thanks for that!
but now I tried adding the "dist/libs" directory to the "moduleDirectories" in the jest config, and using a tsconfig.spec.json with an empty path mapping
"baseUrl": ".", "paths": {},
That seems to be all it takes to get that to run the tests using the compiled artifacts

[yharaskrik]

@gms1 really that is all it took to get it to work? So theoretically if I generate a fresh workspace and make those changes then tests will run with compile artifacts?

[gms1]

after what I have now tested, yes
it also seems logical to me; jest can consume our artifacts like any other library from the node_modules directory and the path mapping is then not required anymore

[yharaskrik]

@gms1 Do you happen to have a sample repo that show this? I am trying to get it working in a fresh repository but it cannot match the paths. Wondering if you have a minimal repo? If I copy my built assets into the node_modules folder it does work as you describe, just not in dist/libs.

It does work if we put them in the dist folder just not dist/libs... Weird

[gms1]

unfortunately, I cannot share the repo that I am currently working on;; hm, in my case it is even in "dist/projects" folder;
trying in a fresh nx workspace now

[yharaskrik]

Amazing! I would love to see a minimal repo so I could recreate it and maybe expand on it in this PR!

[yharaskrik]

Ok it looks like I got it working, I do need to copy my lib artifacts into a separate folder though because they are expecting the repos path prefix @<prefix>/ but libs are build into dist/libs/<lib name>, probably should change the output path to be the same as the ts alias prefix.

[gms1]

aah, you are right in my dist/projects folder I have also the @scope folder

[yharaskrik]

@gms1 I cannot seem to get it to work reliability, make sure while you are testing you are clearing the jest cache prior to running the tests jest --clearCache, otherwise it may be using some jest cached artifacts, when I do that it doesn't seem to find my compiled artifacts.

[gms1]

my problem was rather that I couldn't get the performance improvement I was expecting (it's now around 20% so not bad either), so I'm sure I cleared both the nx and jest caches.
Even so, after resetting the working folder, I was unable to get it to work again until I added a path mapping to the dist folder for all of our shared libraries.

[yharaskrik]

Ok I was able to get it working the solution is somewhere in between my implementation from months ago and you pointing me in the right direction. I will try and do up a PR to show it.

[yharaskrik]

There we go! PR open!

[barbados-clemens]

closing as found in the PR that this doesn't provide as much benefit as expected, unfortunately.

[yharaskrik]

closing as found in the PR that this doesn't provide as much benefit as expected, unfortunately.

Agreed! (I am having deja vu with this conversation though, thought we already closed it haha)

[github-actions]

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.