Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Don't write to fs if not needed
  • Loading branch information
thymikee committed Feb 15, 2017
commit 165e0a6a75b6356aa234b049ea71e6a8598305d5
7 changes: 4 additions & 3 deletions packages/jest-snapshot/src/State.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ class SnapshotState {
snapshotPath?: string,
expand?: boolean,
) {
this._dirty = false;
this._snapshotPath = snapshotPath || getSnapshotPath(testPath);
this._snapshotData = getSnapshotData(this._snapshotPath, update);
const {data, dirty} = getSnapshotData(this._snapshotPath, update);
this._snapshotData = data;
this._dirty = dirty;
this._uncheckedKeys = new Set(Object.keys(this._snapshotData));
this._counters = new Map();
this._index = 0;
Expand Down Expand Up @@ -83,7 +84,7 @@ class SnapshotState {

const isEmpty = Object.keys(this._snapshotData).length === 0;

if ((this._dirty || this._uncheckedKeys.size || update) && !isEmpty) {
if ((this._dirty || this._uncheckedKeys.size) && !isEmpty) {
saveSnapshotFile(this._snapshotData, this._snapshotPath);
status.saved = true;
} else if (isEmpty && fileExists(this._snapshotPath)) {
Expand Down
11 changes: 10 additions & 1 deletion packages/jest-snapshot/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const getSnapshotPath = (testPath: Path) => path.join(
const getSnapshotData = (snapshotPath: Path, update: boolean) => {
const data = Object.create(null);
let snapshotContents;
let dirty = false;

if (fileExists(snapshotPath)) {
try {
Expand All @@ -103,7 +104,15 @@ const getSnapshotData = (snapshotPath: Path, update: boolean) => {
if (!update && snapshotContents) {
validateSnapshotVersion(snapshotContents);
}
return data;

if (update && snapshotContents) {
try {
validateSnapshotVersion(snapshotContents);
} catch (error) {
Copy link
Member

Choose a reason for hiding this comment

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

I kinda feel like we could clean this up to return an error and throw in #105 if an error is returned. Here on #109, instead of try-catch, call the validator and set dirty to true if it returns a value. What do you think as a follow-up?

dirty = true;
}
}
return {data, dirty};
};

// Extra line breaks at the beginning and at the end of the snapshot are useful
Expand Down