Skip to content

Commit

Permalink
RFC: Format snapshot changes ourselves
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 committed Jul 19, 2023
1 parent 2e7a545 commit a7c5129
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 Prettier formatting as a temporary 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 Prettier snapshot 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 a7c5129

Please sign in to comment.