From d9e8e78525e46896125463b965d41c57a4c98038 Mon Sep 17 00:00:00 2001
From: Nokome Bentley <nokome@stenci.la>
Date: Mon, 27 May 2019 15:41:29 +1200
Subject: [PATCH] fix: Lazy reading and async dumping of vfiles

---
 src/gdoc.ts          |  2 +-
 src/html.ts          |  2 +-
 src/index.ts         | 17 +++++++++--------
 src/json.ts          |  2 +-
 src/json5.ts         |  2 +-
 src/pandoc.ts        | 10 ++++++----
 src/tdp.ts           |  4 ++--
 src/vfile.ts         | 26 +++++++++++++++++++++-----
 src/yaml.ts          |  2 +-
 tests/csv.test.ts    |  6 +++---
 tests/gdoc.test.ts   |  2 +-
 tests/html.test.ts   |  2 +-
 tests/pandoc.test.ts |  4 ++--
 tests/tdp.test.ts    |  4 ++--
 14 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/src/gdoc.ts b/src/gdoc.ts
index 62795c130..e6464b4e7 100644
--- a/src/gdoc.ts
+++ b/src/gdoc.ts
@@ -42,7 +42,7 @@ export async function parse(
   file: VFile,
   fetch: boolean = true
 ): Promise<stencila.Node> {
-  const json = dump(file)
+  const json = await dump(file)
   const gdoc = JSON.parse(json)
   return parseDocument(gdoc, fetch)
 }
diff --git a/src/html.ts b/src/html.ts
index 434dbb804..a91300eef 100644
--- a/src/html.ts
+++ b/src/html.ts
@@ -68,7 +68,7 @@ export const mediaTypes = ['text/html']
  * @returns A promise that resolves to a `stencila.Node`
  */
 export async function parse(file: VFile): Promise<stencila.Node> {
-  const html = dump(file)
+  const html = await dump(file)
   const dom = new jsdom.JSDOM(html)
   const document = dom.window.document
   collapse(document)
diff --git a/src/index.ts b/src/index.ts
index ac3f0329f..824282f34 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -299,18 +299,19 @@ export async function write(
 /**
  * Convert content from one format to another.
  *
- * @param inp The input content (raw or file path).
- * @param out The output file path.
+ * @param input The input content (raw or file path).
+ * @param outputPath The output file path.
  * @param options Conversion options e.g `from` and `to`: to specify the formats to convert from/to
  * @returns The converted content, or file path (for converters that only write to files).
  */
 export async function convert(
-  inp: string,
-  out?: string,
+  input: string,
+  outputPath?: string,
   options: { [key: string]: any } = {}
 ): Promise<string | undefined> {
-  const node = await read(inp, options.from)
-  let file = await unparse(node, out, options.to)
-  if (out) await vfile.write(file, out)
-  return file.contents ? vfile.dump(file) : file.path
+  const inputFile = vfile.create(input)
+  const node = await parse(inputFile, input, options.from)
+  const outputFile = await unparse(node, outputPath, options.to)
+  if (outputPath) await vfile.write(outputFile, outputPath)
+  return outputFile.contents ? vfile.dump(outputFile) : outputFile.path
 }
diff --git a/src/json.ts b/src/json.ts
index b1c784a17..2051f3f98 100644
--- a/src/json.ts
+++ b/src/json.ts
@@ -8,7 +8,7 @@ import { dump, load, VFile } from './vfile'
 export const mediaTypes = ['application/json']
 
 export async function parse(file: VFile): Promise<stencila.Node> {
-  return JSON.parse(dump(file))
+  return JSON.parse(await dump(file))
 }
 
 export async function unparse(node: stencila.Node): Promise<VFile> {
diff --git a/src/json5.ts b/src/json5.ts
index 75b185b84..e9295dff9 100644
--- a/src/json5.ts
+++ b/src/json5.ts
@@ -48,7 +48,7 @@ export const mediaTypes = ['application/json5']
  * @returns A promise that resolves to a Stencila `Node`
  */
 export async function parse(file: VFile): Promise<stencila.Node> {
-  return json5.parse(dump(file))
+  return json5.parse(await dump(file))
 }
 
 /**
diff --git a/src/pandoc.ts b/src/pandoc.ts
index f947ba76d..408337c9e 100644
--- a/src/pandoc.ts
+++ b/src/pandoc.ts
@@ -26,11 +26,13 @@ export async function parse(
   ensureFile: boolean = false
 ): Promise<stencila.Node> {
   const args = [`--from=${from}`, `--to=json`].concat(options)
+
   let content = file.contents
   if (!content || ensureFile) {
     if (ensureFile && !file.path) throw new Error('Must supply a file')
     args.push(`${file.path}`)
   }
+
   const json = await run(content, args)
   const pdoc = JSON.parse(json)
   return parseDocument(pdoc)
@@ -62,7 +64,6 @@ export async function unparse(
   const pdoc = unparseArticle(node as stencila.Article)
   await Promise.all(unparsePromises)
 
-  const json = JSON.stringify(pdoc)
   const args = [`--from=json`, `--to=${to}`].concat(options)
   if ((filePath && filePath !== '-') || ensureFile) {
     let output
@@ -74,12 +75,13 @@ export async function unparse(
     args.push(`--output=${output}`)
   }
 
+  const json = JSON.stringify(pdoc)
   const content = await run(json, args)
 
-  // If content was output, then load that into a vfile,
-  // otherwise the vfile, simply has path to the file created
+  // If content was outputted, then load that into a vfile,
+  // otherwise the vfile simply has path to the file created
   if (content) return load(content)
-  else return create({ path: filePath })
+  else return create(undefined, { path: filePath })
 }
 
 /**
diff --git a/src/tdp.ts b/src/tdp.ts
index 30b027fc9..32eac3f2e 100644
--- a/src/tdp.ts
+++ b/src/tdp.ts
@@ -36,7 +36,7 @@ export const extNames = [
 export async function parse(file: VFile): Promise<stencila.Node> {
   let pkg: datapackage.Package
   if (file.path) pkg = await datapackage.Package.load(file.path)
-  else pkg = await datapackage.Package.load(JSON.parse(dump(file)))
+  else pkg = await datapackage.Package.load(JSON.parse(await dump(file)))
 
   // Parse resources
   const parts = await Promise.all(pkg.resources.map(
@@ -199,7 +199,7 @@ async function unparseCreativeWork(
     profile: 'tabular-data-resource',
     name: datatable.name || 'Unnamed',
 
-    data: dump(await csv.unparse(datatable)),
+    data: await dump(await csv.unparse(datatable)),
     format: 'csv',
     mediatype: 'text/csv',
     encoding: 'utf-8',
diff --git a/src/vfile.ts b/src/vfile.ts
index 9db431b10..a33b3a986 100644
--- a/src/vfile.ts
+++ b/src/vfile.ts
@@ -12,9 +12,17 @@ export type VFile = vfile.VFile
 /**
  * Create a virtual file
  *
- * @param options to `vfile` see https://github.com/vfile/vfile#vfileoptions
+ * @param contents Raw string contents of `VFile`, or file path
+ * @param options Options to `vfile` see https://github.com/vfile/vfile#vfileoptions
  */
-export function create(options: any = {}): VFile {
+export function create(
+  contents?: string,
+  options: { [key: string]: any } = {}
+): VFile {
+  if (contents) {
+    if (isPath(contents)) options = { ...options, path: contents }
+    else options = { ...options, contents }
+  }
   return vfile(options)
 }
 
@@ -30,10 +38,18 @@ export function load(contents: VFileContents): VFile {
 /**
  * Dump a string from the contents of a virtual files
  *
+ * If the file has no `contents` but does have a file
+ * `path`, then `read` the virtual file.
+ *
  * @param file The virtual file to dump
  */
-export function dump(vfile: VFile): string {
-  return vfile.contents ? vfile.contents.toString() : ''
+export async function dump(vfile: VFile): Promise<string> {
+  if (vfile.contents) return vfile.toString()
+  if (vfile.path) {
+    const readFile = await toVFile.read(vfile.path)
+    return readFile.toString()
+  }
+  return ''
 }
 
 /**
@@ -66,7 +82,7 @@ export async function write(vfile: VFile, path: string): Promise<void> {
   if (!path) throw new Error('Argument `path` is required')
 
   if (path === '-') {
-    console.log(dump(vfile))
+    console.log(await dump(vfile))
   } else if (path && vfile.contents) {
     vfile.path = path
     await toVFile.write(vfile)
diff --git a/src/yaml.ts b/src/yaml.ts
index 914a36433..9e2ed4edc 100644
--- a/src/yaml.ts
+++ b/src/yaml.ts
@@ -9,7 +9,7 @@ import { dump, load, VFile } from './vfile'
 export const mediaTypes = ['text/yaml']
 
 export async function parse(file: VFile): Promise<stencila.Node> {
-  return yaml.safeLoad(dump(file))
+  return yaml.safeLoad(await dump(file))
 }
 
 export async function unparse(node: stencila.Node): Promise<VFile> {
diff --git a/tests/csv.test.ts b/tests/csv.test.ts
index bf59da5aa..6b4032c8c 100644
--- a/tests/csv.test.ts
+++ b/tests/csv.test.ts
@@ -99,7 +99,7 @@ test('parse', async () => {
 })
 
 test('unparse', async () => {
-  expect(dump(await unparse(simple.node))).toEqual(simple.content)
-  expect(dump(await unparse(named.node))).toEqual(named.content)
-  expect(dump(await unparse(formulas.node))).toEqual(formulas.content)
+  expect(await dump(await unparse(simple.node))).toEqual(simple.content)
+  expect(await dump(await unparse(named.node))).toEqual(named.content)
+  expect(await dump(await unparse(formulas.node))).toEqual(formulas.content)
 })
diff --git a/tests/gdoc.test.ts b/tests/gdoc.test.ts
index 23ffefe48..edc532ee8 100644
--- a/tests/gdoc.test.ts
+++ b/tests/gdoc.test.ts
@@ -7,7 +7,7 @@ test('parse', async () => {
 })
 
 test('unparse', async () => {
-  const u = async (node: any) => JSON.parse(dump(await unparse(node)))
+  const u = async (node: any) => JSON.parse(await dump(await unparse(node)))
   expect(await u(kitchenSink.node)).toEqual(kitchenSink.gdoc)
 })
 
diff --git a/tests/html.test.ts b/tests/html.test.ts
index 1c4822ba0..1a2ec3a36 100644
--- a/tests/html.test.ts
+++ b/tests/html.test.ts
@@ -7,7 +7,7 @@ test('parse', async () => {
 })
 
 test('unparse', async () => {
-  expect(dump(await unparse(kitchenSink.node))).toEqual(kitchenSink.html)
+  expect(await dump(await unparse(kitchenSink.node))).toEqual(kitchenSink.html)
 })
 
 // An example intended for testing progressively added parser/unparser pairs
diff --git a/tests/pandoc.test.ts b/tests/pandoc.test.ts
index e9032915d..09e317d92 100644
--- a/tests/pandoc.test.ts
+++ b/tests/pandoc.test.ts
@@ -22,7 +22,7 @@ test('parse', async () => {
 })
 
 test('unparse', async () => {
-  const u = async (node: any) => JSON.parse(dump(await unparse(node)))
+  const u = async (node: any) => JSON.parse(await dump(await unparse(node)))
   expect(await u(kitchenSink.node)).toEqual(kitchenSink.pdoc)
 })
 
@@ -566,7 +566,7 @@ test('rpngs', async () => {
 
   expect(await parse(load(JSON.stringify(pdoc)))).toEqual(node)
   // Skipping this until resolve how to deal with output RPNG paths
-  //expect(JSON.parse(dump(await unparse(node)))).toEqual(pdoc)
+  //expect(JSON.parse(await dump(await unparse(node)))).toEqual(pdoc)
 })
 
 // A very simple test of the approach to typing Pandoc nodes
diff --git a/tests/tdp.test.ts b/tests/tdp.test.ts
index daf614ceb..62a51a4d1 100644
--- a/tests/tdp.test.ts
+++ b/tests/tdp.test.ts
@@ -137,11 +137,11 @@ test('parse', async () => {
 })
 
 test('unparse', async () => {
-  const actual = JSON.parse(dump(await unparse(periodic.node)))
+  const actual = JSON.parse(await dump(await unparse(periodic.node)))
   // Pretend that we unparsed to a a filePath i.e. that
   // the data was written to disk.
   delete actual.resources[0].data
   actual.resources[0].path = 'periodic-table.csv'
-  const expected = JSON.parse(dump(await read(periodic.file)))
+  const expected = JSON.parse(await dump(await read(periodic.file)))
   expect(actual).toEqual(expected)
 })