Skip to content
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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix(jest-config): accept snapshotFormat.compareKeys option #12387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/jest-config/src/ValidConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 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';
Expand Down Expand Up @@ -110,7 +111,10 @@ const initialOptions: Config.InitialOptions = {
skipFilter: false,
skipNodeResolution: false,
slowTestThreshold: 5,
snapshotFormat: PRETTY_FORMAT_DEFAULTS,
snapshotFormat: {
...PRETTY_FORMAT_DEFAULTS,
compareKeys: () => {},
} as SnapshotFormat,
Comment on lines +114 to +117
Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Member

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 🙂

snapshotResolver: '<rootDir>/snapshotResolver.js',
snapshotSerializers: ['my-serializer-module'],
testEnvironment: 'jest-environment-node',
Expand Down
18 changes: 18 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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';
Expand Down Expand Up @@ -1918,3 +1919,20 @@ describe('updateSnapshot', () => {
Defaults.ci = defaultCiConfig;
});
});

describe('snapshotFormat', () => {
it('should accept custom `compareKeys`', async () => {
const compareKeys = () => 0;

const {options} = await normalize(
{
ci: false,
rootDir: '/root/',
snapshotFormat: {compareKeys} as SnapshotFormat,
Copy link
Contributor Author

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

},
{} as Config.Argv,
);

expect((options.snapshotFormat as any).compareKeys).toBe(compareKeys);
});
});