-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix(jest-config): accept snapshotFormat.compareKeys
option
#12387
Conversation
snapshotFormat: { | ||
...PRETTY_FORMAT_DEFAULTS, | ||
compareKeys: () => {}, | ||
} as SnapshotFormat, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I proud of this fix? No.
Does it work? Yes.
I'm afraid that I won't have time to work on the improved fix though - this is somewhat problematic because this thing is handled through a very generic validation function and this case here is very specific.
If you have any suggestions on the improved design that could be implemented quickly I could take a stab at this but I'm worried that an improved architecture around this might not be a change for a couple of lines of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the long-run plan is to use recently added schemas for validation purposes? This would IMHO go outside of the scope of this PR as it's a bigger refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the long-run plan is to use recently added schemas for validation purposes?
Correct 🙂
{ | ||
ci: false, | ||
rootDir: '/root/', | ||
snapshotFormat: {compareKeys} as SnapshotFormat, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those casts could be removed with such a patch:
diff --git a/packages/jest-config/src/ValidConfig.ts b/packages/jest-config/src/ValidConfig.ts
index db7ea5fe6d..1a4348d5dc 100644
--- a/packages/jest-config/src/ValidConfig.ts
+++ b/packages/jest-config/src/ValidConfig.ts
@@ -5,7 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/
-import type {SnapshotFormat} from '@jest/schemas';
import type {Config} from '@jest/types';
import {replacePathSepForRegex} from 'jest-regex-util';
import {multipleValidOptions} from 'jest-validate';
@@ -113,8 +112,8 @@ const initialOptions: Config.InitialOptions = {
slowTestThreshold: 5,
snapshotFormat: {
...PRETTY_FORMAT_DEFAULTS,
- compareKeys: () => {},
- } as SnapshotFormat,
+ compareKeys: () => 0,
+ },
snapshotResolver: '<rootDir>/snapshotResolver.js',
snapshotSerializers: ['my-serializer-module'],
testEnvironment: 'jest-environment-node',
diff --git a/packages/jest-config/src/__tests__/normalize.test.ts b/packages/jest-config/src/__tests__/normalize.test.ts
index fb229b4e33..7b858656ee 100644
--- a/packages/jest-config/src/__tests__/normalize.test.ts
+++ b/packages/jest-config/src/__tests__/normalize.test.ts
@@ -9,7 +9,6 @@
import {createHash} from 'crypto';
import path from 'path';
import semver = require('semver');
-import type {SnapshotFormat} from '@jest/schemas';
import type {Config} from '@jest/types';
import {escapeStrForRegex} from 'jest-regex-util';
import Defaults from '../Defaults';
@@ -1928,11 +1927,11 @@ describe('snapshotFormat', () => {
{
ci: false,
rootDir: '/root/',
- snapshotFormat: {compareKeys} as SnapshotFormat,
+ snapshotFormat: {compareKeys},
},
{} as Config.Argv,
);
- expect((options.snapshotFormat as any).compareKeys).toBe(compareKeys);
+ expect(options.snapshotFormat.compareKeys).toBe(compareKeys);
});
});
diff --git a/packages/jest-schemas/src/index.ts b/packages/jest-schemas/src/index.ts
index b3f0a32eeb..09a31cff5b 100644
--- a/packages/jest-schemas/src/index.ts
+++ b/packages/jest-schemas/src/index.ts
@@ -10,6 +10,9 @@ import {Static, Type} from '@sinclair/typebox';
const RawSnapshotFormat = Type.Partial(
Type.Object({
callToJSON: Type.Readonly(Type.Boolean()),
+ compareKeys: Type.Readonly(
+ Type.Function([Type.String(), Type.String()], Type.Number()),
+ ),
escapeRegex: Type.Readonly(Type.Boolean()),
escapeString: Type.Readonly(Type.Boolean()),
highlight: Type.Readonly(Type.Boolean()),
But I'm not sure if I should apply it because it feels like maybe compareKeys
was omitted intentionally when declaring schemas in #12384
This won't actually works as we serialize the options so we can pass them to workers/child processes. So functions won't survive. (it should work when running in band, but not in other cases). If we are to support We should update the docs in the meantime so they reflect reality |
Oh, that makes total sense and might explain some weirdness that I've experienced 🙈 In fact... I don't even need this because my use case is simpler. I'd just like to disable the sorting of the property keys. It's important for me to verify the order of the keys because we are creating snapshots of the A sideways note is that... while I've initially attempted to configure this the whole thing has crashed in a not very DX-friendly way. |
Pushed 6baf127 correcting the docs.
Sure. I think I'd prefer overloading the existent function parameter, tho. E.g. supporting some magic string ( |
Codecov Report
@@ Coverage Diff @@
## main #12387 +/- ##
=======================================
Coverage 68.48% 68.48%
=======================================
Files 325 325
Lines 16971 16971
Branches 5061 5061
=======================================
Hits 11622 11622
Misses 5317 5317
Partials 32 32 Continue to review full report at Codecov.
|
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. |
The configuration docs states that:
However, I've found out that it's not possible to configure
snapshotFormat.compareKeys
from withinjest.config.js
file.The reason behind this is that you are outsourcing correct types for options to the
DEFAULT_OPTIONS
from thepretty-format
that are declared here:https://github.com/facebook/jest/blob/c8a84566be774a35a56c26a4595c553df5fc6fc1/packages/pretty-format/src/index.ts#L401
and this is used during validation. But the correct type for
snapshotFormat.compareKeys
doesn't match thetypeof DEFAULT_OPTIONS.compareKeys
.