Skip to content

Commit

Permalink
RFC: Format snapshot changes ourselves (#1222)
Browse files Browse the repository at this point in the history
This misappropriates Jest's reporter functionality so we can plug in to
test file execution and re-format files that have updated snapshots.

Alternative to #1220.
  • Loading branch information
72636c authored Jul 19, 2023
1 parent 2e7a545 commit 674fc94
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 13 deletions.
8 changes: 4 additions & 4 deletions .changeset/nine-keys-accept.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
'skuba': patch
---

test: Disable Prettier snapshot formatting
test: Fix Prettier snapshot formatting

Jest is not yet compatible with Prettier 3, causing snapshot updates to fail with the following error:

Expand All @@ -11,12 +11,12 @@ TypeError: prettier.resolveConfig.sync is not a function
at runPrettier (node_modules/jest-snapshot/build/InlineSnapshots.js:308:30)
```

We have disabled Prettier snapshot formatting in our Jest preset as a temporary workaround. If you do not use the preset, you can manually modify your `jest.config.ts` like so:
Our [Jest preset](https://seek-oss.github.io/skuba/docs/development-api/jest.html#mergepreset) now implements custom formatting as a workaround until [jestjs/jest#14305](https://github.com/jestjs/jest/issues/14305) is resolved.

If you do not use our preset, you can temporarily disable formatting in your `jest.config.ts` then manually run `skuba format` after updating snapshots:

```diff
export default {
+ prettierPath: null,
}
```

You may have to run `skuba format` manually after updating snapshots until [jestjs/jest#14305](https://github.com/jestjs/jest/issues/14305) is resolved.
10 changes: 7 additions & 3 deletions jest-preset.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@ module.exports = {
'!<rootDir>/jest.*.ts',
],
coverageDirectory: 'coverage',
reporters: ['default', require.resolve('./lib/cli/test/reporters/github')],
// jestjs/jest#14305
prettierPath: null,
reporters: [
'default',
require.resolve('./lib/cli/test/reporters/github'),
require.resolve('./lib/cli/test/reporters/prettier'),
],
testEnvironment: 'node',
testPathIgnorePatterns: [
'/node_modules.*/',
'<rootDir>/(coverage|dist|lib|tmp).*/',
],
// jestjs/jest#14305
prettierPath: null,
watchPlugins: [
require.resolve('jest-watch-typeahead/filename'),
require.resolve('jest-watch-typeahead/testname'),
Expand Down
12 changes: 6 additions & 6 deletions src/cli/adapter/prettier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ interface Result {
unparsed: string[];
}

const formatOrLintFile = async (
export const formatOrLintFile = async (
{ data, filepath, options }: File,
mode: 'format' | 'lint',
result: Result,
result: Result | null,
): Promise<string | undefined> => {
if (mode === 'lint') {
let ok: boolean;
Expand All @@ -95,12 +95,12 @@ const formatOrLintFile = async (
(await check(data, options)) &&
(await isPackageJsonOk({ data, filepath }));
} catch (err) {
result.errored.push({ err, filepath });
result?.errored.push({ err, filepath });
return;
}

if (!ok) {
result.errored.push({ filepath });
result?.errored.push({ filepath });
}

return;
Expand All @@ -110,7 +110,7 @@ const formatOrLintFile = async (
try {
formatted = await format(data, options);
} catch (err) {
result.errored.push({ err, filepath });
result?.errored.push({ err, filepath });
return;
}

Expand All @@ -130,7 +130,7 @@ const formatOrLintFile = async (
return;
}

result.touched.push(filepath);
result?.touched.push(filepath);
return formatted;
};

Expand Down
45 changes: 45 additions & 0 deletions src/cli/test/reporters/prettier/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import type { Reporter } from '@jest/reporters';
import fs from 'fs-extra';
import { resolveConfig } from 'prettier';

import { formatOrLintFile } from '../../../../cli/adapter/prettier';

export default class SnapshotPrettifier
implements Pick<Reporter, 'onTestFileResult'>
{
async onTestFileResult(
...[test, testResult]: Parameters<NonNullable<Reporter['onTestFileResult']>>
): Promise<void> {
if (!testResult.snapshot.added && !testResult.snapshot.updated) {
return;
}

const filepath = test.path;

// This is a best-effort workaround to automatically format code.
// Don't pollute console output if it fails for whatever reason.
try {
const [config, data] = await Promise.all([
resolveConfig(filepath),
fs.promises.readFile(filepath, 'utf-8'),
]);

const formatted = await formatOrLintFile(
{
data,
filepath,
options: {
...config,
filepath,
},
},
'format',
null,
);

if (typeof formatted === 'string') {
await fs.promises.writeFile(filepath, formatted);
}
} catch {}
}
}

0 comments on commit 674fc94

Please sign in to comment.