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

Jump to bottom

fix(jest-snapshot): Fix a potential bug when using babel and improve performance #14036

Merged
merged 3 commits into from Mar 31, 2023
Merged

fix(jest-snapshot): Fix a potential bug when using babel and improve performance #14036

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca7b1f0
fix
liuxingbaoyu Mar 29, 2023
cb7d126
fix lint
liuxingbaoyu Mar 29, 2023
877842b
changelog
liuxingbaoyu Mar 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@

- `[jest-environment-jsdom, jest-environment-node]` Fix assignment of `customExportConditions` via `testEnvironmentOptions` when custom env subclass defines a default value ([#13989](https://github.com/facebook/jest/pull/13989))
- `[jest-matcher-utils]` Fix copying value of inherited getters ([#14007](https://github.com/facebook/jest/pull/14007))
- `[jest-snapshot]` Fix a potential bug when not using prettier and improve performance ([#14007](https://github.com/facebook/jest/pull/14007))

### Chore & Maintenance

Expand Down
2 changes: 0 additions & 2 deletions packages/jest-snapshot/package.json
View file
Open in desktop
Expand Up @@ -21,12 +21,10 @@
"@babel/generator": "^7.7.2",
"@babel/plugin-syntax-jsx": "^7.7.2",
"@babel/plugin-syntax-typescript": "^7.7.2",
"@babel/traverse": "^7.7.2",
"@babel/types": "^7.3.3",
"@jest/expect-utils": "workspace:^",
"@jest/transform": "workspace:^",
"@jest/types": "workspace:^",
"@types/babel__traverse": "^7.0.6",
"@types/prettier": "^2.1.5",
"babel-preset-current-node-syntax": "^1.0.0",
"chalk": "^4.0.0",
Expand Down
235 changes: 118 additions & 117 deletions packages/jest-snapshot/src/InlineSnapshots.ts
View file
Open in desktop
Expand Up @@ -7,7 +7,13 @@

import * as path from 'path';
import type {ParseResult, PluginItem} from '@babel/core';
import type {Expression, File, Program} from '@babel/types';
import type {
Expression,
File,
Node,
Program,
TraversalAncestors,
} from '@babel/types';
import * as fs from 'graceful-fs';
import type {
CustomParser as PrettierCustomParser,
Expand All @@ -17,17 +23,18 @@
import type {Frame} from 'jest-message-util';
import {escapeBacktickString} from './utils';

// prettier-ignore
const babelTraverse = (
// @ts-expect-error requireOutside Babel transform
requireOutside('@babel/traverse') as typeof import('@babel/traverse')
).default;
// prettier-ignore
const generate = (
// @ts-expect-error requireOutside Babel transform
requireOutside('@babel/generator') as typeof import('@babel/generator')
).default;
const {file, isAwaitExpression, templateElement, templateLiteral} =
const {
isAwaitExpression,
templateElement,
templateLiteral,
traverse,
traverseFast,
} =
// @ts-expect-error requireOutside Babel transform
requireOutside('@babel/types') as typeof import('@babel/types');
// @ts-expect-error requireOutside Babel transform
Expand Down Expand Up @@ -135,17 +142,22 @@
// substitute in the snapshots in reverse order, so slice calculations aren't thrown off.
const sourceFileWithSnapshots = snapshots.reduceRight(
(sourceSoFar, nextSnapshot) => {
const {node} = nextSnapshot;
if (
!nextSnapshot.node ||
typeof nextSnapshot.node.start !== 'number' ||
typeof nextSnapshot.node.end !== 'number'
!node ||
typeof node.start !== 'number' ||
typeof node.end !== 'number'
) {
throw new Error('Jest: no snapshot insert location found');
}

// A hack to prevent unexpected line breaks in the generated code
node.loc!.end.line = node.loc!.start.line;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually only this line is the fix, the rest are refactorings.😃


return (
sourceSoFar.slice(0, nextSnapshot.node.start) +
generate(nextSnapshot.node, {retainLines: true}).code.trim() +
sourceSoFar.slice(nextSnapshot.node.end)
sourceSoFar.slice(0, node.start) +
generate(node, {retainLines: true}).code.trim() +
sourceSoFar.slice(node.end)
);
},
sourceFile,
Expand Down Expand Up @@ -214,69 +226,56 @@
.join('\n');
};

const resolveAst = (fileOrProgram: any): File => {
// Flow uses a 'Program' parent node, babel expects a 'File'.
let ast = fileOrProgram;
if (ast.type !== 'File') {
ast = file(ast, ast.comments, ast.tokens);
delete ast.program.comments;
}
return ast;
};

const traverseAst = (
snapshots: Array<InlineSnapshot>,
fileOrProgram: File | Program,
ast: File | Program,
snapshotMatcherNames: Array<string>,
) => {
const ast = resolveAst(fileOrProgram);
const groupedSnapshots = groupSnapshotsByFrame(snapshots);
const remainingSnapshots = new Set(snapshots.map(({snapshot}) => snapshot));

babelTraverse(ast, {
CallExpression({node}) {
const {arguments: args, callee} = node;
if (
callee.type !== 'MemberExpression' ||
callee.property.type !== 'Identifier' ||
callee.property.loc == null
) {
return;
}
const {line, column} = callee.property.loc.start;
const snapshotsForFrame = groupedSnapshots[`${line}:${column}`];
if (!snapshotsForFrame) {
return;
}
if (snapshotsForFrame.length > 1) {
throw new Error(
'Jest: Multiple inline snapshots for the same call are not supported.',
);
}

snapshotMatcherNames.push(callee.property.name);
traverseFast(ast, function (node: Node) {

Check failure on line 237 in packages/jest-snapshot/src/InlineSnapshots.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected function expression
if (node.type !== 'CallExpression') return;

const snapshotIndex = args.findIndex(
({type}) => type === 'TemplateLiteral',
const {arguments: args, callee} = node;
if (
callee.type !== 'MemberExpression' ||
callee.property.type !== 'Identifier' ||
callee.property.loc == null
) {
return;
}
const {line, column} = callee.property.loc.start;
const snapshotsForFrame = groupedSnapshots[`${line}:${column}`];
if (!snapshotsForFrame) {
return;
}
if (snapshotsForFrame.length > 1) {
throw new Error(
'Jest: Multiple inline snapshots for the same call are not supported.',
);
const values = snapshotsForFrame.map(inlineSnapshot => {
inlineSnapshot.node = node;
const {snapshot} = inlineSnapshot;
remainingSnapshots.delete(snapshot);

return templateLiteral(
[templateElement({raw: escapeBacktickString(snapshot)})],
[],
);
});
const replacementNode = values[0];

if (snapshotIndex > -1) {
args[snapshotIndex] = replacementNode;
} else {
args.push(replacementNode);
}
},
}
const inlineSnapshot = snapshotsForFrame[0];
inlineSnapshot.node = node;

snapshotMatcherNames.push(callee.property.name);

const snapshotIndex = args.findIndex(
({type}) => type === 'TemplateLiteral',
);

const {snapshot} = inlineSnapshot;
remainingSnapshots.delete(snapshot);
const replacementNode = templateLiteral(
[templateElement({raw: escapeBacktickString(snapshot)})],
[],
);

if (snapshotIndex > -1) {
args[snapshotIndex] = replacementNode;
} else {
args.push(replacementNode);
}
});

if (remainingSnapshots.size) {
Expand Down Expand Up @@ -340,59 +339,61 @@
// Workaround for https://github.com/prettier/prettier/issues/3150
options.parser = inferredParser;

const ast = resolveAst(parsers[inferredParser](text, options));
babelTraverse(ast, {
CallExpression({node: {arguments: args, callee}, parent}) {
if (
callee.type !== 'MemberExpression' ||
callee.property.type !== 'Identifier' ||
!snapshotMatcherNames.includes(callee.property.name) ||
!callee.loc ||
callee.computed
) {
return;
}
const ast = parsers[inferredParser](text, options);
traverse(ast, function (node: Node, ancestors: TraversalAncestors) {

Check failure on line 343 in packages/jest-snapshot/src/InlineSnapshots.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected function expression
if (node.type !== 'CallExpression') return;

let snapshotIndex: number | undefined;
let snapshot: string | undefined;
for (let i = 0; i < args.length; i++) {
const node = args[i];
if (node.type === 'TemplateLiteral') {
snapshotIndex = i;
snapshot = node.quasis[0].value.raw;
}
}
if (snapshot === undefined || snapshotIndex === undefined) {
return;
const {arguments: args, callee} = node;
if (
callee.type !== 'MemberExpression' ||
callee.property.type !== 'Identifier' ||
!snapshotMatcherNames.includes(callee.property.name) ||
!callee.loc ||
callee.computed
) {
return;
}

let snapshotIndex: number | undefined;
let snapshot: string | undefined;
for (let i = 0; i < args.length; i++) {
const node = args[i];
if (node.type === 'TemplateLiteral') {
snapshotIndex = i;
snapshot = node.quasis[0].value.raw;
}
}
if (snapshot === undefined) {
return;
}

const startColumn =
isAwaitExpression(parent) && parent.loc
? parent.loc.start.column
: callee.loc.start.column;

const useSpaces = !options.useTabs;
snapshot = indent(
snapshot,
Math.ceil(
useSpaces
? startColumn / (options.tabWidth ?? 1)
: // Each tab is 2 characters.
startColumn / 2,
),
useSpaces ? ' '.repeat(options.tabWidth ?? 1) : '\t',
);

const replacementNode = templateLiteral(
[
templateElement({
raw: snapshot,
}),
],
[],
);
args[snapshotIndex] = replacementNode;
},
const parent = ancestors[ancestors.length - 1].node;
const startColumn =
isAwaitExpression(parent) && parent.loc
? parent.loc.start.column
: callee.loc.start.column;

const useSpaces = !options.useTabs;
snapshot = indent(
snapshot,
Math.ceil(
useSpaces
? startColumn / (options.tabWidth ?? 1)
: // Each tab is 2 characters.
startColumn / 2,
),
useSpaces ? ' '.repeat(options.tabWidth ?? 1) : '\t',
);

const replacementNode = templateLiteral(
[
templateElement({
raw: snapshot,
}),
],
[],
);
args[snapshotIndex!] = replacementNode;
});

return ast;
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-snapshot/src/utils.ts
View file
Open in desktop
Expand Up @@ -194,7 +194,7 @@ const printBacktickString = (str: string): string =>

export const ensureDirectoryExists = (filePath: string): void => {
try {
fs.mkdirSync(path.join(path.dirname(filePath)), {recursive: true});
fs.mkdirSync(path.dirname(filePath), {recursive: true});
} catch {}
};

Expand Down
4 changes: 1 addition & 3 deletions yarn.lock
View file
Open in desktop
Expand Up @@ -1832,7 +1832,7 @@ __metadata:
languageName: node
linkType: hard

"@babel/traverse@npm:^7.12.9, @babel/traverse@npm:^7.18.8, @babel/traverse@npm:^7.20.0, @babel/traverse@npm:^7.20.10, @babel/traverse@npm:^7.20.12, @babel/traverse@npm:^7.20.13, @babel/traverse@npm:^7.20.5, @babel/traverse@npm:^7.20.7, @babel/traverse@npm:^7.7.2":
"@babel/traverse@npm:^7.12.9, @babel/traverse@npm:^7.18.8, @babel/traverse@npm:^7.20.0, @babel/traverse@npm:^7.20.10, @babel/traverse@npm:^7.20.12, @babel/traverse@npm:^7.20.13, @babel/traverse@npm:^7.20.5, @babel/traverse@npm:^7.20.7":
version: 7.20.13
resolution: "@babel/traverse@npm:7.20.13"
dependencies:
Expand Down Expand Up @@ -13096,15 +13096,13 @@ __metadata:
"@babel/plugin-syntax-typescript": ^7.7.2
"@babel/preset-flow": ^7.7.2
"@babel/preset-react": ^7.12.1
"@babel/traverse": ^7.7.2
"@babel/types": ^7.3.3
"@jest/expect-utils": "workspace:^"
"@jest/test-utils": "workspace:^"
"@jest/transform": "workspace:^"
"@jest/types": "workspace:^"
"@tsd/typescript": ^4.9.0
"@types/babel__core": ^7.1.14
"@types/babel__traverse": ^7.0.6
"@types/graceful-fs": ^4.1.3
"@types/natural-compare": ^1.4.0
"@types/prettier": ^2.1.5
Expand Down