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

feat: add writeJson/writeJsonSync for fs modules #271

Merged
merged 5 commits into from
Mar 14, 2019
Merged

feat: add writeJson/writeJsonSync for fs modules #271

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e6003d
feat: add writeJson/writeJsonSync for fs modules
axetroy Mar 13, 2019
ef73f2e
styles: follow the style guide
axetroy Mar 14, 2019
bb51bce
Rename WriteJsonOption to WriteJsonOptions
kitsonk Mar 14, 2019
9523765
refactor: reanme WriteJsonOption to WriteJsonOptions
axetroy Mar 14, 2019
61f3da7
fix: eslint error
axetroy Mar 14, 2019
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
79 changes: 79 additions & 0 deletions fs/write_json.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
import * as path from "./path/mod.ts";

interface WriteJsonOption {
axetroy marked this conversation as resolved.
Show resolved Hide resolved
spaces?: number | string;
replacer?: (number | string)[] | null | Replacer;
axetroy marked this conversation as resolved.
Show resolved Hide resolved
}

type Replacer = (key: string, value: any) => any;
axetroy marked this conversation as resolved.
Show resolved Hide resolved

/**
* Writes an object to a JSON file.
* @export
* @param {string} filePath
* @param {*} object
* @param {WriteJsonOption} [options]
* @returns {Promise<void>}
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these docs superfluous given type annotations? CC @kitsonk

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. We shouldn't be using JSDoc like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Without revelant descriptions of parameters?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to provide descriptions you can do that, this JSDoc is not doing that. If providing descriptions is informative then the following would be acceptable:

/** Some description
 * @param filePath The path to the file
 */

What is included here is not acceptable. Need to find out if there is an eslint rule for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

you mean this one: https://eslint.org/docs/rules/valid-jsdoc ?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, because those include the types... we want something that does not include the types.

@j-f1 any rule you know of to keep types out of JSDoc?

Copy link
Contributor

@zekth zekth Mar 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@kitsonk you may look this option: https://eslint.org/docs/rules/valid-jsdoc#requireparamtype . If set to false it fits your need.

Copy link

Choose a reason for hiding this comment

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

gajus/eslint-plugin-jsdoc#134

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right. But adding enforce of JsDoc good atm? And when this feature will be integrated we add this config to eslint no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this interface is obvious and there is no need to explain the parameters.
So I only leave a single line comment

*/
export async function writeJson(
filePath: string,
object: any,
options?: WriteJsonOption
axetroy marked this conversation as resolved.
Show resolved Hide resolved
): Promise<void> {
filePath = path.resolve(filePath);

let contentRaw: string = "";

try {
if (options) {
contentRaw = JSON.stringify(
object,
(options.replacer as string[]) || null,
options.spaces
);
} else {
contentRaw = JSON.stringify(object);
}
} catch (err) {
err.message = `${filePath}: ${err.message}`;
throw err;
}

await Deno.writeFile(filePath, new TextEncoder().encode(contentRaw));
}

/**
* Writes an object to a JSON file.
* @export
* @param {string} filePath
* @param {*} object
* @param {WriteJsonOption} [options]
* @returns {void}
*/
export function writeJsonSync(
filePath: string,
object: any,
options?: WriteJsonOption
): void {
filePath = path.resolve(filePath);

let contentRaw: string = "";

try {
if (options) {
contentRaw = JSON.stringify(
object,
(options.replacer as string[]) || null,
options.spaces
);
} else {
contentRaw = JSON.stringify(object);
}
} catch (err) {
err.message = `${filePath}: ${err.message}`;
throw err;
}

Deno.writeFileSync(filePath, new TextEncoder().encode(contentRaw));
}
244 changes: 244 additions & 0 deletions fs/write_json_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
import { test } from "../testing/mod.ts";
import {
assertEquals,
assertThrowsAsync,
assertThrows
} from "../testing/asserts.ts";
import { writeJson, writeJsonSync } from "./write_json.ts";
import * as path from "./path/mod.ts";

const testdataDir = path.resolve("fs", "testdata");

test(async function writeJsonIfNotExists() {
const notExistsJsonFile = path.join(testdataDir, "file_not_exists.json");

await assertThrowsAsync(
async () => {
await writeJson(notExistsJsonFile, { a: "1" });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = await Deno.readFile(notExistsJsonFile);

await Deno.remove(notExistsJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(async function writeJsonIfExists() {
const existsJsonFile = path.join(testdataDir, "file_write_exists.json");

await Deno.writeFile(existsJsonFile, new Uint8Array());

await assertThrowsAsync(
async () => {
await writeJson(existsJsonFile, { a: "1" });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = await Deno.readFile(existsJsonFile);

await Deno.remove(existsJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(async function writeJsonIfExistsAnInvalidJson() {
const existsInvalidJsonFile = path.join(
testdataDir,
"file_write_invalid.json"
);

const invalidJsonContent = new TextEncoder().encode("[123}");
await Deno.writeFile(existsInvalidJsonFile, invalidJsonContent);

await assertThrowsAsync(
async () => {
await writeJson(existsInvalidJsonFile, { a: "1" });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = await Deno.readFile(existsInvalidJsonFile);

await Deno.remove(existsInvalidJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(async function writeJsonWithSpaces() {
const existsJsonFile = path.join(testdataDir, "file_write_spaces.json");

const invalidJsonContent = new TextEncoder().encode();
await Deno.writeFile(existsJsonFile, invalidJsonContent);

await assertThrowsAsync(
async () => {
await writeJson(existsJsonFile, { a: "1" }, { spaces: 2 });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = await Deno.readFile(existsJsonFile);

await Deno.remove(existsJsonFile);

assertEquals(new TextDecoder().decode(content), `{\n "a": "1"\n}`);
});

test(async function writeJsonWithReplacer() {
const existsJsonFile = path.join(testdataDir, "file_write_replacer.json");

const invalidJsonContent = new TextEncoder().encode();
await Deno.writeFile(existsJsonFile, invalidJsonContent);

await assertThrowsAsync(
async () => {
await writeJson(
existsJsonFile,
{ a: "1", b: "2", c: "3" },
{
replacer: ["a"]
}
);
throw new Error("should write success");
},
Error,
"should write success"
);

const content = await Deno.readFile(existsJsonFile);

await Deno.remove(existsJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(function writeJsonSyncIfNotExists() {
const notExistsJsonFile = path.join(testdataDir, "file_not_exists_sync.json");

assertThrows(
() => {
writeJsonSync(notExistsJsonFile, { a: "1" });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = Deno.readFileSync(notExistsJsonFile);

Deno.removeSync(notExistsJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(function writeJsonSyncIfExists() {
const existsJsonFile = path.join(testdataDir, "file_write_exists_sync.json");

Deno.writeFileSync(existsJsonFile, new Uint8Array());

assertThrows(
() => {
writeJsonSync(existsJsonFile, { a: "1" });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = Deno.readFileSync(existsJsonFile);

Deno.removeSync(existsJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(function writeJsonSyncIfExistsAnInvalidJson() {
const existsInvalidJsonFile = path.join(
testdataDir,
"file_write_invalid_sync.json"
);

const invalidJsonContent = new TextEncoder().encode("[123}");
Deno.writeFileSync(existsInvalidJsonFile, invalidJsonContent);

assertThrows(
() => {
writeJsonSync(existsInvalidJsonFile, { a: "1" });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = Deno.readFileSync(existsInvalidJsonFile);

Deno.removeSync(existsInvalidJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});

test(function writeJsonWithSpaces() {
const existsJsonFile = path.join(testdataDir, "file_write_spaces_sync.json");

const invalidJsonContent = new TextEncoder().encode();
Deno.writeFileSync(existsJsonFile, invalidJsonContent);

assertThrows(
() => {
writeJsonSync(existsJsonFile, { a: "1" }, { spaces: 2 });
throw new Error("should write success");
},
Error,
"should write success"
);

const content = Deno.readFileSync(existsJsonFile);

Deno.removeSync(existsJsonFile);

assertEquals(new TextDecoder().decode(content), `{\n "a": "1"\n}`);
});

test(function writeJsonWithReplacer() {
const existsJsonFile = path.join(
testdataDir,
"file_write_replacer_sync.json"
);

const invalidJsonContent = new TextEncoder().encode();
Deno.writeFileSync(existsJsonFile, invalidJsonContent);

assertThrows(
() => {
writeJsonSync(
existsJsonFile,
{ a: "1", b: "2", c: "3" },
{
replacer: ["a"]
}
);
throw new Error("should write success");
},
Error,
"should write success"
);

const content = Deno.readFileSync(existsJsonFile);

Deno.removeSync(existsJsonFile);

assertEquals(new TextDecoder().decode(content), `{"a":"1"}`);
});
1 change: 1 addition & 0 deletions test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import "./fs/ensure_dir_test.ts";
import "./fs/ensure_file_test.ts";
import "./fs/move_test.ts";
import "./fs/read_json_test.ts";
import "./fs/write_json_test.ts";
import "./io/test.ts";
import "./http/server_test.ts";
import "./http/file_server_test.ts";
Expand Down