Fix issue with default value in workspaceState/globalState

[daimor]

What it does

Fixes issues described in #8435
The Issue with parsing JSON happens just after writeJSON with correct and valid data, but the file is very empty. Replacement to writeJSONSync solves the issue.

Another issue with getting value from workspaceState/globalState, if previously updated with undefined. Original Memento suppose to return defaultValue if no previous value or undefined.

How to test

Examples provided in the linked issue
Test code

			// Just clear start, completely nothing in globalState, yet
			val = globalState.get('test', "default") || "WTF";

			globalState.update('test1', undefined);
			val1 = globalState.get('test1', "default") || "WTF";

			globalState.update('test2', "somevalue");
			val2 = globalState.get('test2', "default") || "WTF";	

Expected values,

val="default"
val1="default"
val2="somevalue"

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[akosyakov]

we should avoid to use sync method, since it blocks the backend.

[akosyakov]

Do you think we can replace it with fs.read and fs.write to have it async but creating proper not empty content?

[daimor]

Yeah, just looking now, how to avoid sync but still keep right file content.

[daimor]

Found the core of the issue
https://stackoverflow.com/questions/30886217/node-js-fs-writefile-empties-the-file
so, I've changed it to save asynchronously to a temporary file (just suffix .tmp), and rename to the original file. And it solved the issue. Wanted to add a test, but did not manage to reproduce the issue there.

[akosyakov]

👍

[paul-marechal]

Callback shouldn't be resolve alone but something like

error => {
  if (error) {
    reject(error);
  } else {
    resolve();
  }
}

Alternatively you could use Node's promises fs apis if you find it simplier.

[akosyakov]

the solution looks strange to me. There should be a way to call only one fs method. Maybe don't use fs-extra but fs module directly and use JSON.stringify to serialise?

[daimor]

I've tried already a few options, and this way in fact does not also. It fails with the error, .tmp files does not exist.
fs.promises, does not help as well.

The main issue actually not here, but in subsequent read, and it happens when OS did not finish an actual write, and the file is completely empty. I think the best way would be to use write to disk only as a way to keep file up to date, but read it only if have not changed since our last read. And even do not write if we have not changed anything.

[daimor]

Decided to keep only one change here, about getting default value from globalState/workspaceState.

The issue with an empty json file needs some re-work, and have to be solved separately.

[akosyakov]

thank you!