misc improvements (#385)

* watch mode doesn't work; change script back to one-shot

* normalize paths and resolve ~ in paths

* clean up types, enable @ts-check in more places

* fix schema push and pull commands

also adds a few types and another test

package.json

@@ -120,7 +120,7 @@
     "pretest": "yarn fixlint",
     "local": "export $(cat .env | xargs); node bin/run",
     "local-test-old": "export $(cat .env | xargs); mocha \"test/**/*.test.{js,ts}\"",
-    "local-test": "mocha --watch --recursive ./yargs-test --require ./yargs-test/mocha-root-hooks.mjs --experimental-require-module",
+    "local-test": "mocha --recursive ./yargs-test --require ./yargs-test/mocha-root-hooks.mjs",
     "lint": "eslint .",
     "fixlint": "eslint . --fix",
     "test-old": "c8 -r html mocha --forbid-only \"test/**/*.test.{js,ts}\"",

src/cli.mjs

@@ -7,7 +7,7 @@ import evalCommand from "./yargs-commands/eval.mjs";
 import loginCommand from "./yargs-commands/login.mjs";
 import schemaCommand from "./yargs-commands/schema/schema.mjs";
 import databaseCommand from "./yargs-commands/database.mjs";
-import { logArgv } from "./lib/middleware.mjs";
+import { logArgv, fixPaths } from "./lib/middleware.mjs";
 
 /** @typedef {import('awilix').AwilixContainer<import('./config/setup-container.mjs').modifiedInjectables>} cliContainer */
 
@@ -17,8 +17,7 @@ export let container;
 export let builtYargs;
 
 /**
- * @function run
- * @param {string} argvInput - The command string provided by the user or test. Parsed by yargs into an argv object.
+ * @param {string|string[]} argvInput - The command string provided by the user or test. Parsed by yargs into an argv object.
  * @param {cliContainer} _container - A built and ready for use awilix container with registered injectables.
  */
 export async function run(argvInput, _container) {
@@ -40,16 +39,18 @@ export async function run(argvInput, _container) {
   }
 }
 
-// we split this out so it can be injected/mocked
-// this lets us record calls against it and, e.g.,
-// ensure it's only run once per command invocation
+/**
+ * This is split out so it can be injected & spied on. This allows ensuring that,
+ * e.g., it's only run once per command invocation.
+ * @param {import('yargs').Argv<any>} builtYargs
+ * @returns {Promise<any>} builtYargs
+ */
 export async function parseYargs(builtYargs) {
   return builtYargs.parseAsync();
 }
 
 /**
- * @function buildYargs
- * @param {string} argvInput
+ * @param {string|string[]} argvInput
  * @returns {import('yargs').Argv<any>}
  */
 function buildYargs(argvInput) {
@@ -61,6 +62,7 @@ function buildYargs(argvInput) {
     yargsInstance
       .scriptName("fauna")
       .middleware([logArgv], true)
+      .middleware([fixPaths], false)
       .command("eval", "evaluate a query", evalCommand)
       .command("login", "login via website", loginCommand)
       .command(schemaCommand)

src/config/setup-container.mjs

@@ -23,6 +23,9 @@ import open from "open";
 import OAuthClient from "../lib/auth/oauth-client.mjs";
 import { Lifetime } from "awilix";
 import fs from "node:fs";
+import * as fsp from "node:fs/promises";
+import path from "node:path";
+import os from "node:os";
 import { AccountKey } from "../lib/file-util.mjs";
 import { parseYargs } from "../cli.mjs";
 
@@ -47,11 +50,15 @@ export function setupCommonContainer() {
  */
 
 /** @typedef {Resolvers<injectables>} modifiedInjectables */
+
 export const injectables = {
   // node libraries
   exit: awilix.asValue(exit),
   fetch: awilix.asValue(fetchWrapper),
   fs: awilix.asValue(fs),
+  fsp: awilix.asValue(fsp),
+  normalize: awilix.asValue(path.normalize),
+  homedir: awilix.asValue(os.homedir),
 
   // third-party libraries
   confirm: awilix.asValue(confirm),
@@ -82,7 +89,7 @@ export const injectables = {
 };
 
 export function setupRealContainer() {
-  /** @type {awilix.AwilixContainer<injectables>} */
+  /** @type {awilix.AwilixContainer<modifiedInjectables>} */
   const container = setupCommonContainer();
 
   container.register(injectables);

src/config/setup-test-container.mjs

@@ -1,7 +1,9 @@
 import fs from "node:fs";
+import { normalize } from "node:path";
 
 import * as awilix from "awilix/lib/awilix.module.mjs";
 import { setupCommonContainer, injectables } from "./setup-container.mjs";
+import { f } from "../../yargs-test/helpers.mjs";
 
 import { stub, spy } from "sinon";
 import { parseYargs } from "../cli.mjs";
@@ -42,6 +44,10 @@ export function setupTestContainer() {
     // real implementation
     parseYargs: awilix.asValue(spy(parseYargs)),
     fs: awilix.asValue(stub(fs)),
+    fsp: awilix.asValue({
+      unlink: stub(),
+      writeFile: stub(),
+    }),
     logger: awilix.asValue({
       // use these for making dev, support tickets easier.
       // they're not mocked because we shouldn't test them
@@ -71,6 +77,8 @@ export function setupTestContainer() {
       error.code = exitCode;
       throw error;
     }),
+    normalize: awilix.asValue(spy(normalize)),
+    fetch: awilix.asValue(stub().resolves(f({}))),
   };
 
   confirmManualMocks(manualMocks, thingsToManuallyMock);

src/lib/db.mjs

@@ -3,15 +3,22 @@
 import { container } from "../cli.mjs";
 
 /**
- * @function makeFaunaRequest
- * @param {object} args
- * @param {string} args.secret - The secret to include in the AUTHORIZATION header of the request.
- * @param {string} args.baseUrl - The base URL from the scheme up through the top level domain and optional port; defaults to "https://db.fauna.com:443".
- * @param {string} args.path - The path part of the URL. Added to the baseUrl and params to build the full URL.
- * @param {Record<string, string>} [args.params] - The parameters (and their values) to set in the query string.
- * @param {('GET'|'HEAD'|'OPTIONS'|'PATCH'|'PUT'|'POST'|'DELETE'|'PATCH')} args.method - The HTTP method to use when making the request.
- * @param {object} [args.body] - The body to include in the request.
- * @param {boolean} [args.shouldThrow] - Whether or not to throw if the network request succeeds but is not a 2XX. If this is set to false, makeFaunaRequest will return the error instead of throwing.
+ * @typedef {('GET'|'HEAD'|'OPTIONS'|'PATCH'|'PUT'|'POST'|'DELETE'|'PATCH')} method
+ */
+
+/**
+ * @typedef {Object} fetchParameters
+ * @property {string} secret - The secret to include in the AUTHORIZATION header of the request.
+ * @property {string} baseUrl - The base URL from the scheme up through the top level domain and optional port; defaults to "https://db.fauna.com:443".
+ * @property {string} path - The path part of the URL. Added to the baseUrl and params to build the full URL.
+ * @property {Record<string, string>} [params] - The parameters (and their values) to set in the query string.
+ * @property {method} method - The HTTP method to use when making the request.
+ * @property {object} [body] - The body to include in the request.
+ * @property {boolean} [shouldThrow=true] - Whether or not to throw if the network request succeeds but is not a 2XX. If this is set to false, makeFaunaRequest will return the error instead of throwing.
+ */
+
+/**
+ * @param {fetchParameters} args
  */
 export async function makeFaunaRequest({
   secret,

src/lib/file-util.mjs

@@ -1,14 +1,27 @@
 //@ts-check
 
 import fs from "node:fs";
-import { normalize } from "node:path";
-import * as os from "node:os";
+import os from "node:os";
 import { container } from "../cli.mjs";
 
-// path: string, returns boolean
+/**
+ * Fixes paths by normalizing them (.. => parent directory) and resolving ~ to homedir.
+ * @param {string} path - The path to fix.
+ * @returns {string}
+ */
+export function fixPath(path) {
+  const normalize = container.resolve("normalize");
+  const homedir = container.resolve("homedir");
+  return normalize(path.replace(/^~/, homedir));
+}
+
+/**
+ * @function dirExists
+ * @param {string} path - The path to check existence for.
+ * @returns {boolean}
+ */
 export function dirExists(path) {
-  // TODO: needs to resolve home dir (~); node path libs won't do that for us
-  const stat = fs.statSync(normalize(path), {
+  const stat = fs.statSync(fixPath(path), {
     // returns undefined instead of throwing if the file doesn't exist
     throwIfNoEntry: false,
   });
@@ -19,10 +32,14 @@ export function dirExists(path) {
   }
 }
 
-// path: string, returns boolean
+/**
+ * @function dirIsWriteable
+ * @param {string} path - The path to check existence for.
+ * @returns {boolean}
+ */
 export function dirIsWriteable(path) {
   try {
-    fs.accessSync(path, fs.constants.W_OK);
+    fs.accessSync(fixPath(path), fs.constants.W_OK);
   } catch (e) {
     return false;
   }
@@ -38,7 +55,7 @@ export function dirIsWriteable(path) {
  */
 function fileExists(path) {
   try {
-    fs.readFileSync(path);
+    fs.readFileSync(fixPath(path));
     return true;
   } catch (e) {
     return false;
@@ -56,7 +73,6 @@ export class Credentials {
    */
   constructor(filename = "") {
     this.logger = container.resolve("logger");
-    this.exit = container.resolve("exit");
     this.filename = filename;
     this.credsDir = `${os.homedir()}/.fauna/credentials`;
     if (!dirExists(this.credsDir)) {

src/lib/middleware.mjs

@@ -1,9 +1,18 @@
 //@ts-check
 
 import { container } from "../cli.mjs";
+import { fixPath } from "../lib/file-util.mjs";
 
 export function logArgv(argv) {
   const logger = container.resolve("logger");
   logger.debug(JSON.stringify(argv, null, 4), "argv", argv);
   return argv;
 }
+
+export function fixPaths(argv) {
+  if (argv.dir) {
+    return { ...argv, dir: fixPath(argv.dir) };
+  } else {
+    return argv;
+  }
+}

src/lib/schema.mjs

@@ -1,8 +1,13 @@
+//@ts-check
+
 import * as path from "path";
 import { dirExists, dirIsWriteable } from "./file-util.mjs";
 import { container } from "../cli.mjs";
 import { makeFaunaRequest } from "../lib/db.mjs";
 
+/**
+ * @param {string} dir - The directory path to check for existence and write access
+ */
 function checkDirUsability(dir) {
   if (!dirExists(dir)) {
     throw new Error(`The project fsl directory: ${dir} does not exist.`);
@@ -11,10 +16,15 @@ function checkDirUsability(dir) {
   }
 }
 
-// Reads the files using their relative-to-`basedir` paths and returns their
-// contents paired with the relative path.
-// Fails if the total size of the files is too large.
-// relpaths: string[]
+/**
+ * Reads files using their relative-to-`dir` paths and returns their contents
+ * paired with their relative paths. Fails if the total size of the files
+ * is too large.
+ *
+ * @param {string} dir - The path to the root directory the FSL files are stored in
+ * @param {string[]} relpaths - A list of paths (relative to `dir`) to individual FSL files
+ * @returns {LocalFSLFileDescription[]}
+ */
 function read(dir, relpaths) {
   const fs = container.resolve("fs");
   const logger = container.resolve("logger");
@@ -39,12 +49,14 @@ function read(dir, relpaths) {
   return curr;
 }
 
-// Gathers all FSL files in the directory rooted at `basedir` and returns a
-// list of relative paths.
-// Fails if there are too many files.
-// returns string[]
+/**
+ * Gathers all FSL files in the directory rooted at `dir` and returns a list of
+ * relative paths. Fails if there are too many files.
+ *
+ * @param {string} dir - The path to the directory to delete unused FSL files from
+ * @returns {Promise<string[]>}
+ */
 export async function gatherRelativeFSLFilePaths(dir) {
-  const logger = container.resolve("logger");
   const fs = container.resolve("fs");
 
   const FILE_LIMIT = 32000;
@@ -70,21 +82,36 @@ export async function gatherRelativeFSLFilePaths(dir) {
   };
   const files = go("", []);
   if (files.length > FILE_LIMIT) {
-    logger.stderr(`Too many files: ${files.length} > ${FILE_LIMIT}`);
+    throw new Error(`Too many files: ${files.length} > ${FILE_LIMIT}`);
   }
   return files;
 }
 
+/**
+ * @param {string} dir - The path to the directory to delete unused FSL files from
+ * @param {string[]} filesToDelete - A dictionary of filenames to their contents
+ * @returns {Promise<void>}
+ */
 export async function deleteUnusedSchemaFiles(dir, filesToDelete) {
-  const fs = container.resolve("fs");
+  const fsp = container.resolve("fsp");
   const promises = [];
   for (const fileName of filesToDelete) {
-    promises.push(fs.unlink(path.join(dir, fileName)));
+    promises.push(fsp.unlink(path.join(dir, fileName)));
   }
 
-  return Promise.all(promises);
+  await Promise.all(promises);
 }
 
+/**
+ * @typedef LocalFSLFileDescription
+ * @property {string} name
+ * @property {string} content
+ */
+
+/**
+ * @param {string} dir - The path to the directory to gather FSL files from
+ * @returns {Promise<LocalFSLFileDescription[]>}
+ */
 export async function gatherFSL(dir) {
   const gatherRelativeFSLFilePaths = container.resolve(
     "gatherRelativeFSLFilePaths"
@@ -93,64 +120,90 @@ export async function gatherFSL(dir) {
   checkDirUsability(dir);
   const fps = await gatherRelativeFSLFilePaths(dir);
   const files = read(dir, fps);
-  return JSON.stringify(files);
+  return files;
 }
 
-export async function writeSchemaFiles(dir, filenameToContentsHash) {
+/**
+ * @param {string} dir - The path to the directory to write FSL files to
+ * @param {Record<string, string>} filenameToContentsDict - A dictionary of filenames to their contents
+ * @returns {Promise<void>}
+ */
+export async function writeSchemaFiles(dir, filenameToContentsDict) {
   const fs = container.resolve("fs");
+  const fsp = container.resolve("fsp");
   fs.mkdirSync(path.dirname(dir), { recursive: true });
 
   const promises = [];
   for (const [filename, fileContents] of Object.entries(
-    filenameToContentsHash
+    filenameToContentsDict
   )) {
     const fp = path.join(dir, filename);
-    promises.push(fs.writeFile(fp, fileContents));
+    promises.push(fsp.writeFile(fp, fileContents));
   }
 
-  return Promise.all(promises);
+  await Promise.all(promises);
 }
 
-export async function getAllSchemaFileContents(
-  filenames,
-  { ...overrides } = {}
-) {
+/** @typedef {import('./db.mjs').fetchParameters} fetchParameters */
+
+/**
+ * @param {string[]} filenames - A list of schema file names to fetch
+ * @param {Omit<fetchParameters, "path"|"method">} overrides
+ * @returns {Promise<Record<string, string>>} A map of schema file names to their contents.
+ */
+export async function getAllSchemaFileContents(filenames, { ...overrides }) {
   const promises = [];
-  const fileContents = {};
+  /* @type Record<string, string> */
+  const fileContentCollection = {};
   for (const filename of filenames) {
     promises.push(
-      getSchemaFile(filename, overrides).then((fileContent) => {
-        fileContents[filename] = fileContent;
+      getSchemaFile(filename, overrides).then(({ content }) => {
+        fileContentCollection[filename] = content;
       })
     );
   }
 
-  return Promise.all(promises);
+  await Promise.all(promises);
+
+  return fileContentCollection;
 }
 
-export async function getSchemaFiles({ ...overrides } = {}) {
+/**
+ * @param {Omit<fetchParameters, "path"|"method">} overrides
+ */
+export async function getSchemaFiles({ ...overrides }) {
+  /** @type {fetchParameters} */
   const args = {
+    ...overrides,
     path: "/schema/1/files",
     method: "GET",
-    ...overrides,
   };
   return makeFaunaRequest({ ...args });
 }
 
-export async function getSchemaFile(filename, { ...overrides } = {}) {
+/**
+ * @param {string} filename
+ * @param {Omit<fetchParameters, "path"|"method">} overrides
+ */
+export async function getSchemaFile(filename, { ...overrides }) {
+  /** @type {fetchParameters} */
   const args = {
+    ...overrides,
     path: `/schema/1/files/${encodeURIComponent(filename)}`,
     method: "GET",
-    ...overrides,
   };
   return makeFaunaRequest({ ...args });
 }
 
-export async function getStagedSchemaStatus({ ...overrides } = {}) {
+/**
+ * @param {Omit<fetchParameters, "path"|"method">} overrides
+ */
+export async function getStagedSchemaStatus({ ...overrides }) {
+  /** @type {fetchParameters} */
   const args = {
+    ...overrides,
     path: "/schema/1/staged/status",
     method: "GET",
-    ...overrides,
   };
   return makeFaunaRequest({ ...args });
 }

src/yargs-commands/eval.mjs

@@ -69,10 +69,8 @@ async function writeFormattedOutput(file, data, format) {
  * @param {boolean} [flags.typecheck] - (Optional) Flag to enable typechecking
  */
 export async function performQuery(client, fqlQuery, outputFile, flags) {
-  console.log("query!");
   if (flags.version === "4") {
     const res = performV4Query(client, fqlQuery, outputFile, flags);
-    console.log(res);
     return res;
   } else {
     return performV10Query(client, fqlQuery, outputFile, flags);

src/yargs-commands/schema/pull.mjs

@@ -41,7 +41,7 @@ async function doPull(argv) {
     .sort();
 
   // Gather local .fsl files to overwrite or delete.
-  const existing = await gatherFSL(argv.dir);
+  const existing = (await gatherFSL(argv.dir)).map((file) => file.name);
 
   // Summarize file changes.
   const adds = [];
@@ -62,7 +62,7 @@ async function doPull(argv) {
   }
   deletes.sort();
 
-  logger.stdout("Pull makes the following changes:");
+  logger.stdout("Pull will make the following changes:");
   if (argv.delete) {
     for (const deleteme of deletes) {
       logger.stdout(`delete:    ${deleteme}`);

yargs-entrypoint.mjs

@@ -1,7 +1,9 @@
 #!/usr/bin/env node
 
+//@ts-check
+
 import { hideBin } from "yargs/helpers";
 import { run } from "./src/cli.mjs";
 import { setupRealContainer as setupContainer } from "./src/config/setup-container.mjs";
 
-run(hideBin(process.argv), await setupContainer());
+run(hideBin(process.argv), setupContainer());

yargs-test/schema/diff.mjs

@@ -104,4 +104,25 @@ describe("schema diff", function () {
     expect(logger.stdout).to.have.been.calledWith("No schema differences");
     expect(logger.stderr).to.not.have.been.called;
   });
+
+  it("can parse relative paths", async function () {
+    const gatherFSL = container.resolve("gatherFSL");
+
+    await run(
+      `schema diff --secret "secret" --dir /all/but/the/leaf/..`,
+      container
+    );
+
+    expect(gatherFSL).to.have.been.calledWith("/all/but/the");
+  });
+
+  it("can parse home directory paths", async function () {
+    const gatherFSL = container.resolve("gatherFSL");
+    const homedir = container.resolve("homedir");
+    homedir.returns("/Users/test-user");
+
+    await run(`schema diff --secret "secret" --dir ~`, container);
+
+    expect(gatherFSL).to.have.been.calledWith("/Users/test-user");
+  });
 });

yargs-test/schema/pull.mjs

@@ -42,9 +42,13 @@ describe("schema pull", function () {
 
   it("can pull schema", async function () {
     const gatherFSL = container.resolve("gatherFSL");
-    gatherFSL.resolves(
-      '[{"name":"coll.fsl","content":"collection MyColl {\\n  name: String\\n  index byName {\\n    terms [.name]\\n  }\\n}\\n"}]'
-    );
+    gatherFSL.resolves([
+      {
+        name: "coll.fsl",
+        content:
+          "collection MyColl {\\n  name: String\\n  index byName {\\n    terms [.name]\\n  }\\n}\\n",
+      },
+    ]);
 
     // user accepts the changes in the interactive prompt
     confirm.resolves(true);
@@ -127,7 +131,7 @@ describe("schema pull", function () {
     expect(logger.stdout).to.have.been.calledWith("add:       second.fsl");
     expect(logger.stdout).to.have.been.calledWith("add:       third.fsl");
     expect(logger.stdout).to.have.been.calledWith(
-      "Pull makes the following changes:"
+      "Pull will make the following changes:"
     );
     // expect(writeSchemaFiles).to.have.been.calledWith([{
     // }])
@@ -139,7 +143,7 @@ describe("schema pull", function () {
 
   it("can be cancelled by the user without modifying the filesystem", async function () {
     const gatherFSL = container.resolve("gatherFSL");
-    gatherFSL.resolves("");
+    gatherFSL.resolves([]);
 
     // user rejects the changes in the interactive prompt
     confirm.resolves(false);
@@ -167,7 +171,7 @@ describe("schema pull", function () {
     expect(logger.stdout).to.have.been.calledWith("add:       second.fsl");
     expect(logger.stdout).to.have.been.calledWith("add:       third.fsl");
     expect(logger.stdout).to.have.been.calledWith(
-      "Pull makes the following changes:"
+      "Pull will make the following changes:"
     );
     expect(logger.stdout).to.have.been.calledWith("Change cancelled");
     expect(fs.writeFile).to.have.not.been.called;

yargs-test/schema/push.mjs

@@ -16,7 +16,6 @@ describe("schema push", function () {
 
   it("can force push schema", async function () {
     const fetch = container.resolve("fetch");
-    fetch.resolves({ json: async () => ({}) });
 
     const gatherFSL = container.resolve("gatherFSL");
     gatherFSL.resolves(
@@ -48,5 +47,16 @@ describe("schema push", function () {
 
   it.skip("can be cancelled by the user before making mutating network calls", async function () {});
 
+  it("can push schema from another directory", async function () {
+    const gatherFSL = container.resolve("gatherFSL");
+
+    await run(
+      `schema push --secret "secret" --force --dir "/absolute/path/elsewhere"`,
+      container
+    );
+
+    expect(gatherFSL).to.have.been.calledWith("/absolute/path/elsewhere");
+  });
+
   it.skip("warns when attempting to push an empty diff", async function () {});
 });