-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
JestBuilder not respect globals properties defined in jest.config.js #1059
Comments
same problem here. I want to set nx/packages/builders/src/jest/jest.builder.ts Lines 50 to 55 in 2ce53ea
In case anyone else has the exact problem as me: until it is possible to configure the globals in the builder, a workaround is to add the following to the "jest": {
"globals": {
"ts-jest": {
"skipBabel": true
}
}
}, and then running |
I noticed that when I comment: nx/packages/builders/src/jest/jest.builder.ts Lines 50 to 55 in 2ce53ea
|
Same issue I am facing here while trying to migrate to jest-preset-angular alpha-2, some new configurations needed to be set in the globals and the builder override it. globals need to be extensible. #thymikee/jest-preset-angular#217 |
Instead of defining globals here: nx/packages/builders/src/jest/jest.builder.ts Lines 50 to 55 in 2ce53ea
cli will generate this lines inside jest.config.ts ?
|
Hey guys there are many issues lately which seem unrelated but they are - nx isn't playing nice with Can someone share a working config: |
I'd like to get this to work as well. The reason it does not work is that |
Hey @FrozenPandaz Thanks for looking into this, we've been going round in circles for days. Some of the stuff we've tried:
I thought that would force it to work with the old AND new APIs, but sadly that doesn't work either. So it's probably not an API thing.
|
The stringification may not be a problem - see the migration notes for jest-preset-angular. The globals should be overridable, I think that's how the preset does it (override/extend rather than replace). |
@lonix1
If you want this to work in your build server, this is another story as you have to download the tar file for @nrwl/builders and patch it, then use your local version for installation. Let me know if you need help with this. |
@mohyeid Thanks... Do you mean this line? Nope that doesn't work for me I still can't get tests to run. Maybe we are using different versions, or different config files. I also tried commenting out the stuff mentioned in the posts above. Hopefully if someone gets this to work, we can get a set of working configs that we can look at. Or a minimal sample repo. |
I'm on the latest version.
Version 7.1.1 that you're using is "ancient" in that many bugs have been fixed since then. Is there a reason you're still using it? Anyways, unfortunately, even using the latest doesn't resolve the issue. :( |
@FrozenPandaz, did you try to move this config to generated jest.config.ts? |
I have not, but have some thoughts: Caveats:
Alternatives:
|
Being able to modify After migrating from Karma, there was an incorrect test running successfully, which definitely shouldn't. Currently, I find no possibility to set |
Are there any plans in future version of nx to make the |
I ended up adding this patch using patch-package: diff --git a/node_modules/@nrwl/jest/src/builders/jest/jest.impl.js b/node_modules/@nrwl/jest/src/builders/jest/jest.impl.js
index 1f2e0e5..4d6a7a5 100644
--- a/node_modules/@nrwl/jest/src/builders/jest/jest.impl.js
+++ b/node_modules/@nrwl/jest/src/builders/jest/jest.impl.js
@@ -18,7 +18,7 @@ function run(options, context) {
// Typechecking wasn't done in Jest 23 but is done in 24. This makes errors a warning to amend the breaking change for now
// Remove for v8 to fail on type checking failure
diagnostics: {
- warnOnly: true
+ warnOnly: false
}
};
// TODO: This is hacky, We should probably just configure it in the user's workspace I realise that it was a breaking change but v8 is now out and I really can't wait. We use TS for a reason (: |
I am interested in this issue as well. At my company, we recently added the
With these changes, you could even invoke jest directly as the jest config would have all the information needed to run. If this seems like a good approach let me know and I can make a PR implementing these. |
Having been assigned to this, I'd like to get some clarity on the right direction to head with the fix. I'm inclined to agree with @ZachJW34 that the @FrozenPandaz (or anyone invested in this) - can you see any reason this wouldn't work? I'll admit I'm fairly new to this project so there may be nuances I've missed. One thing that keeps needling at me is that the builder config is in JSON but the config file is JS so the overrides couldn't have the same potential (e.g. no functions) which suggests a global Update : (I'm not sure I should post when I haven't woken up yet. 🤪)
The more I look at this issue, the more I wonder whether this shouldn't be a bug/feature request against Jest itself... |
Hi @jdpearce , thx for the PR. I was waiting for this feature since a long time. Sadly there is still a small problem. Since the property const globals = {
'ts-jest': { diagnostics: false, isolatedModules: true },
}
const defaultConfig = {
tsConfig: '../tsconfig.spec.json',
stringifyContentPathRegex: '\\.(html|svg)$',
astTransformers: ['jest-preset-angular/InlineHtmlStripStylesTransformer']
}
// does not work
const mergeBroken = Object.assign(globals, {
'ts-jest': defaultConfig
});
// works
const merge = Object.assign(globals, {
'ts-jest': Object.assign(defaultConfig, globals['ts-jest'] || {})
}); |
It does appear as though this bug is still in full force. Because the "overrides" from angular.json are overwriting the entire The current approach is a brute-force override, rather than a merger of the two So, DON'T do this: // merge the jestConfig globals with our 'ts-jest' override
const jestConfig: { globals: any } = require(options.jestConfig);
const globals = jestConfig.globals || {};
Object.assign(globals, {
'ts-jest': tsJestConfig
}); DO this: // merge the jestConfig globals with our 'ts-jest' override
const jestConfig: { globals: any } = require(options.jestConfig);
const globals = jestConfig.globals || {};
Object.assign(globals, {
'ts-jest': {
...(globals['ts-jest'] || {}),
...tsJestConfig
}
}); I have made this small change to the jest builder code in my node_modules, and it seems to work. |
@jrista @jdpearce waiting for this feature, so I have just update to Nx 8.6.0 but this still doesn't seem to work. This is the globals section in my config in jest.config.js in the root of my repo, watch the isolatedModules.
now when logging what is coming inside of the jest.imp.js file for a library 'shared-utils'
gives me:
This is the jest.config.js file in the root of this library. Which has a preset on the root jest.config.js. But I don't see any code which merges this root jest.config.js into the jestConfig. Jest has a normalize file which contains logic to resolve the preset property in jest.config.js. Shouldn't this be applied into the builder to make this work? |
@evtk: Looking at your examples there...it looks like you are logging the jestConfig before it has been merged. The code I fixed used to look like this: const jestConfig: { globals: any } = require(options.jestConfig);
const globals = jestConfig.globals || {};
Object.assign(globals, {
'ts-jest': tsJestConfig
}); I updated it to the following, but I've marked where you are (I assume) logging: const jestConfig: { globals: any } = require(options.jestConfig);
// *** YOU ARE LOGGING HERE ***
const globals = jestConfig.globals || {};
Object.assign(globals, {
'ts-jest': {
...(globals['ts-jest'] || {}),
...tsJestConfig
}
}); So you are logging before the fix actually merges your custom globals config with the defaults that the jest builder grabs from config. The key to know if you are using the latest code is to simply check if you have my fix or not. If you do not have my fix, which is the double spread there, then it may just be that a version of the nrwl builder for jest that has the fix has not yet been published. |
@jrista thanks for checking in. I do have the updated code it seems. Now I'm logging below:
but logging this, still doesn't give me the isolatedModules: true configuration which I have present in my jest.config.js file in the root of the angular repo.
Could you try this out yourself? Am I missing some piece of the puzzle? :) |
I think I'm seeing the same issue as @evtk although I may be misunderstanding how this is supposed to work. It looks like the preset defined in each library's jest.config file is not respected. If I define my global override in my library config everything works perfectly but it's not picked up if the override is defined in the root config file. I've got 40+ libraries so I'd rather not have to update the config in each library. |
Hi folks. I am looking at this now and it seems to me that this is technically a Jest problem. Jest is afaict responsible for loading up the There are some special cases, but I believe if you put something in Is it worth raising an issue against Jest itself? Or maybe someone could create a PR similar to the one @jrista raised for Nx? To fix this in Nx we would have to do Jest's work for it, which is feasible, but seems like the wrong way to go, imo. |
I raised an issue with Jest and created a PR to resolve it. If anyone wants to watch how that goes, they're here : jestjs/jest#9026 |
Brilliant. Thanks for doing that. Much appreciated. |
If Jest itself is doing something to override, then that might still be a problem. I did update the unit tests for nrwl/jest builder when I made the fix, so the builder itself should be properly merging globals config (at least, whatever such config the builder works with). It seems to work for my one project locally, but...I don't think I actually have a situation where a child jest.config.js has globals as well as the root jest.config.js. So I may not actually have encountered the scenario @evtk and @IDonut are experiencing. |
The PR I raised with Jest has been merged! Hopefully this will solve any problems people are having with the preset globals being clobbered. @evtk - I'm not sure when Jest will release the change, but please update and try it out when they do! |
I have this global setting added to the workspace root // In <workspace-roo>/jest.config.js
module.exports = {
testMatch: ['**/+(*.)+(spec).+(ts|js)?(x)'],
...
globals: {
'ts-jest': {
diagnostics: false,
},
},
} $ npx nx report
@nrwl/angular : Not Found
@nrwl/cli : 8.8.3
@nrwl/cypress : Not Found
@nrwl/eslint-plugin-nx : Not Found
@nrwl/express : Not Found
@nrwl/jest : 8.8.3
@nrwl/linter : 8.8.3
@nrwl/nest : 8.8.3
@nrwl/next : Not Found
@nrwl/node : 8.8.3
@nrwl/react : Not Found
@nrwl/schematics : Not Found
@nrwl/tao : 8.8.3
@nrwl/web : Not Found
@nrwl/workspace : 8.8.3
typescript : 3.5.3 |
Hi @demisx, Jest hasn't released a new version with the fix for that problem yet. You'll have to keep an eye on their release schedule to know when it will be out. We should be updating our dependency shortly after they release. |
I can report back that after jest has released this fix, it now finally works as we all expected. Thanks for assistance! |
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. |
Expected Behavior
Properties defined in
globals
property insidejest.config.js
should be available in jest context.Current Behavior
At this moment
globals
is ignored because hereJestBuilder
set own globals settings so Jest ignores settings fromjest.config.js
.This behavior occures when I run tests via
ng test
. When I run asjest
or inside WebStorm IDE tests work fine.Failure Information (for bugs)
I set
enableTsDiagnostics
property to enable type-checking inside my tests, but it's not work.Steps to Reproduce
ng test
.Context
Please provide any relevant information about your setup:
angular.json
configurationI prepare repo with bug reproduction. In my case I'd like to enable type checking in
ts-jest
so I need to setenableTsDiagnostics
flag totrue
.The text was updated successfully, but these errors were encountered: