]> git.wincent.com - wincent.git/commitdiff
refactor(fig): use Path string-like type
authorGreg Hurrell <greg@hurrell.net>
Sun, 19 Apr 2020 12:01:32 +0000 (14:01 +0200)
committerGreg Hurrell <greg@hurrell.net>
Sun, 19 Apr 2020 12:01:32 +0000 (14:01 +0200)
I might regret doing this, but the idea here is to replace code that
looks like:

    import {join, resolve} from 'path';

    const file = resource('a');
    const somePath = resolve(join(file, '..', 'x'));

with:

    const file = resource('a');
    const somePath = file.join('..', 'x').resolve();

Other examples in tests along the lines of:

    file.basename
    file.dirname
    file.resolve
    file.join('x')

I bet there are remaining bugs though; as you can see, I had to pepper
the code base with `toString()` calls before passing these to Node APIs
like `path.join()` because it complains with:

    TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of
    type string. Received type object

Which is more annoying than anything else, to be honest, but it is what
it is... Sadly, type system here doesn't help us because these methods
take type "string", and my "Path" strings are that type...

aspects/dotfiles/index.ts
package.json
src/Compiler.ts
src/Fig/__tests__/resource-test.ts [new file with mode: 0644]
src/Fig/resource.ts
src/__tests__/stringify-test.ts
src/path/absolute.ts
src/path/expand.ts
src/path/simplify.ts
src/stringify.ts

index 8fde07cb3e55e3fee5d736ec950b9793ce51260a..5b15158c34a901a0d23ddfdf0da462cd6deaed16 100644 (file)
@@ -1,4 +1,4 @@
-import {basename, join} from 'path';
+import {join} from 'path';
 
 import expand from '../../src/path/expand.js';
 
@@ -45,9 +45,9 @@ task('copy to ~/backups', async () => {
     const files = resource.files('*');
 
     for (const file of files) {
-        const base = basename(file);
+        const base = file.basename;
         const source = expand(`~/${base}`);
-        const target = join(expand('~/.backups'), base);
+        const target = join(expand('~/.backups'), base.toString());
 
         const stats = await stat(source);
 
@@ -69,7 +69,7 @@ task('create symlinks', async () => {
     for (const src of files) {
         await file({
             force: true,
-            path: expand(`~/${basename(src)}`),
+            path: expand(`~/${src.basename}`),
             src,
             state: 'link',
         });
index 6d7dfa3c8ed062bc0a3a71b60a8eea2303b371c2..393b4f51b76529520cc478ae57bec196fe862df9 100644 (file)
@@ -9,8 +9,8 @@
     "license": "Unlicense",
     "scripts": {
         "ci": "yarn format:check",
-        "format:check": "npx prettier --check \"**/*.{js,json,ts}\" \"*.md\" roles/dotfiles/files/.zsh/liferay/bin/portool",
-        "format": "npx prettier --write \"**/*.{js,json,ts}\" \"*.md\" roles/dotfiles/files/.zsh/liferay/bin/portool"
+        "format:check": "npx prettier --check \"**/*.{js,json,ts}\" \"*.md\" aspects/dotfiles/files/.zsh/liferay/bin/portool",
+        "format": "npx prettier --write \"**/*.{js,json,ts}\" \"*.md\" aspects/dotfiles/files/.zsh/liferay/bin/portool"
     },
     "type": "module",
     "dependencies": {
index d497fec84ab54e47703b8394e6b40b84ceb838e7..5b8b60026a6c36260551a87f883cbfc88e5cc96f 100644 (file)
@@ -15,6 +15,8 @@ export default class Compiler {
     }
 
     async compile(path: string): Promise<{fill: (scope: Scope) => string}> {
+        path = path.toString(); // Convert Path string-like back to primitive.
+
         const map = this.#compiled;
 
         if (!map.has(path)) {
diff --git a/src/Fig/__tests__/resource-test.ts b/src/Fig/__tests__/resource-test.ts
new file mode 100644 (file)
index 0000000..b077852
--- /dev/null
@@ -0,0 +1,97 @@
+import {join} from 'path';
+
+import {describe, expect, test} from '../../test/harness.js';
+import Context from '../Context.js';
+import {file, files, template} from '../resource.js';
+
+function withMeta(callback: () => void) {
+    return () => {
+        try {
+            Context.currentAspect = 'meta';
+            callback();
+        } finally {
+            delete Context.currentAspect;
+        }
+    };
+}
+
+const inspect = Symbol.for('nodejs.util.inspect.custom');
+
+describe('file()', () => {
+    test(
+        'returns a relative path',
+        withMeta(() => {
+            expect(file('example.txt').toString()).toBe(
+                'aspects/meta/files/example.txt'
+            );
+        })
+    );
+
+    test('returns a string-like object', () => {
+        const example: any = file('example.txt');
+
+        expect(example instanceof String).toBe(true);
+        expect(Object.prototype.toString.call(example)).toBe('[object String]');
+        expect(example[inspect]()).toBe('aspects/meta/files/example.txt');
+    });
+
+    test('returns a value with a basename property', () => {
+        const example: any = file('example.txt');
+
+        expect(example.basename.toString()).toBe('example.txt');
+    });
+
+    test('returns a value with a dirname property', () => {
+        const example: any = file('example.txt');
+
+        expect(example.dirname.toString()).toBe('aspects/meta/files');
+    });
+
+    test('returns a value with a resolve property', () => {
+        const example: any = file('example.txt');
+
+        expect(example.resolve.toString()).toBe(
+            join(process.cwd(), 'aspects/meta/files/example.txt')
+        );
+    });
+
+    test('returns a value with a join property', () => {
+        const example: any = file('example.txt');
+
+        // Note that normalization (simplification of ".." components) is
+        // automatic.
+        expect(example.join('..', 'foo').toString()).toBe(
+            'aspects/meta/files/foo'
+        );
+    });
+
+    test('returns a value with chainable helper properties', () => {
+        const example: any = file('example.txt');
+
+        expect(example.resolve.dirname.join('other.txt').toString()).toBe(
+            join(process.cwd(), 'aspects/meta/files/other.txt')
+        );
+    });
+});
+
+describe('files()', () => {
+    test(
+        'returns relative paths',
+        withMeta(() => {
+            expect(files('*').map((f) => f.toString())).toEqual([
+                'aspects/meta/files/example.txt',
+            ]);
+        })
+    );
+});
+
+describe('template()', () => {
+    test(
+        'returns a relative path',
+        withMeta(() => {
+            expect(template('sample.txt.erb').toString()).toBe(
+                'aspects/meta/templates/sample.txt.erb'
+            );
+        })
+    );
+});
index 439800e54b6dc4ea200a97a60db34f94aab305d7..376bca373c6b7daeb0ac607cf8eb165422a0c91c 100644 (file)
@@ -1,25 +1,35 @@
 import * as fs from 'fs';
-import {join} from 'path';
+import {basename, dirname, join, normalize, resolve} from 'path';
 
 import Context from './Context.js';
 import globToRegExp from './globToRegExp.js';
 
 // TODO: think about exporting these separately
 
+const inspect = Symbol.for('nodejs.util.inspect.custom');
+
+type Path = string & {
+    basename: Path;
+    dirname: Path;
+    join: Path;
+    resolve: Path;
+
+    // Makes sure value as printed by `console.log()` looks like a string.
+    [inspect]: () => string;
+};
+
 /**
  * Given `name` returns an aspect-local path corresponding to the currently
  * active aspect (eg. `aspects/${aspect}/files/${name}`).
  */
-export function file(...path: Array<string>): string {
-    return join('aspects', Context.currentAspect, 'files', ...path);
+export function file(...path: Array<string>): Path {
+    return toPath(join('aspects', Context.currentAspect, 'files', ...path));
 }
-// TODO: consider returing Path instead
-// (basically string & {basename: string} etc convenience methods)
 
 /**
  * Very simple glob-based file search (doesn't supported nested directories).
  */
-export function files(glob: string): Array<string> {
+export function files(glob: string): Array<Path> {
     const aspect = Context.currentAspect;
 
     const regExp = globToRegExp(glob);
@@ -29,9 +39,41 @@ export function files(glob: string): Array<string> {
         .filter((entry) => entry.isDirectory() || entry.isFile())
         .map(({name}) => name)
         .filter((name) => regExp.test(name))
-        .map((name) => join('aspects', aspect, 'files', name));
+        .map((name) => toPath(join('aspects', aspect, 'files', name)));
+}
+
+export function template(...path: Array<string>): Path {
+    return toPath(join('aspects', Context.currentAspect, 'templates', ...path));
 }
 
-export function template(...path: Array<string>): string {
-    return join('aspects', Context.currentAspect, 'templates', ...path);
+function toPath(string: string): Path {
+    return Object.defineProperties(new String(string), {
+        basename: {
+            get() {
+                return toPath(basename(string));
+            },
+        },
+
+        dirname: {
+            get() {
+                return toPath(dirname(string));
+            },
+        },
+
+        join: {
+            value: (...components: Array<string>) => {
+                return toPath(normalize(join(string, ...components)));
+            },
+        },
+
+        resolve: {
+            get() {
+                return toPath(resolve(string));
+            },
+        },
+
+        [inspect]: {
+            value: () => string,
+        },
+    });
 }
index 2db529922cc63feeaae8955a8af61a4e204f485d..dd20b773f413fddb1a2d1225cb3dcfca8ec2df83 100644 (file)
@@ -26,6 +26,10 @@ test('stringify() a string', () => {
     expect(stringify('thing')).toBe('"thing"');
 });
 
+test('stringify() a String', () => {
+    expect(stringify(new String('thing'))).toBe('"thing"');
+});
+
 test('stringify() a Symbol', () => {
     expect(stringify(Symbol.for('sample'))).toBe('Symbol(sample)');
 });
index d78e58785f635e793ad2cac94b942a206ad8879b..54c2f653b704cc9876be96c53305716bf8b1b3b0 100644 (file)
@@ -2,6 +2,7 @@ import {resolve} from 'path';
 
 import expand from './expand.js';
 
+// TODO: probably put "expand" on Path-likes as well
 export default function absolute(path: string) {
-    return resolve(expand(path));
+    return resolve(expand(path.toString()));
 }
index 050494c8fdfe19c8104fbbd45a7d882a2506e3b7..bcc1f499a7a08e0d357b7c573724d5bb70b146c4 100644 (file)
@@ -5,6 +5,6 @@ export default function expand(path: string) {
     if (path.startsWith('~/')) {
         return join(homedir(), path.slice(2));
     } else {
-        return path;
+        return path.toString();
     }
 }
index b95ba15d3b9bfabe9b58971525e9bf29e9c60e72..7f18de0dc0ab7c10302f0047962fdf1ed3abd957 100644 (file)
@@ -7,6 +7,6 @@ export default function simplify(path: string) {
     if (path.startsWith(home)) {
         return join('~', path.slice(home.length));
     } else {
-        return relative('', path);
+        return relative('', path.toString());
     }
 }
index 40ea289f2554dc7ed6991dcab8ae75d6c6e9e5f3..9c36259b75e3671f3fee401877dcc5a9d8a34368 100644 (file)
@@ -2,6 +2,10 @@ import {LAQUO, RAQUO} from './Unicode.js';
 
 const CIRCULAR = `${LAQUO}circular${RAQUO}`;
 
+// TODO: consider using this as a fallback for objects which define a custom
+// inspect method.
+// const inspect = Symbol.for('nodejs.util.inspect.custom');
+
 /**
  * Basically `JSON.stringify()` but does a better job of printing some value
  * types (eg. a `RegExp` is printed as "/pattern/" instead of "{}" etc).
@@ -20,7 +24,7 @@ export default function stringify(value: unknown) {
             value instanceof RegExp
         ) {
             return String(value);
-        } else if (typeof value === 'string') {
+        } else if (typeof value === 'string' || value instanceof String) {
             return JSON.stringify(value);
         } else if (value instanceof Error) {
             return JSON.stringify(value.toString());