fix: hardcoded '/' in normalized options.fs, fixes #412, #408

Changes including:
- will not call fs.resolve when normalizing `fs`
- removed hardcoded `/`
- mandatory `fs.dirname` for relative reference
- mandatory `fs.sep`, defaults to '/'

src/fs/browser.ts

@@ -60,3 +60,9 @@ export async function exists (filepath: string) {
 export function existsSync (filepath: string) {
   return true
 }
+
+export function dirname (filepath: string) {
+  return domResolve(filepath, '.')
+}
+
+export const sep = '/'

src/fs/fs.ts

@@ -1,8 +1,20 @@
+import { LoaderOptions } from './loader'
+
 export interface FS {
+  /** check if a file exists asynchronously */
   exists: (filepath: string) => Promise<boolean>;
-  readFile: (filepath: string) => Promise<string>;
+  /** check if a file exists synchronously */
   existsSync: (filepath: string) => boolean;
+  /** read a file asynchronously */
+  readFile: (filepath: string) => Promise<string>;
+  /** read a file synchronously */
   readFileSync: (filepath: string) => string;
-  resolve: (root: string, file: string, ext: string) => string;
+  /** resolve a file against directory, for given `ext` option */
+  resolve: (dir: string, file: string, ext: string, options?: LoaderOptions) => string;
+  /** defaults to "/", will be used for "within roots" check */
+  sep?: string;
+  /** dirname for a filepath, used when resolving relative path */
+  dirname?: (file: string) => string;
+  /** fallback file for lookup failure */
   fallback?: (file: string) => string | undefined;
 }

src/fs/loader.ts

@@ -1,6 +1,8 @@
 import { FS } from './fs'
+import { escapeRegex } from '../util/underscore'
+import { assert } from '../util/assert'
 
-interface LoaderOptions {
+export interface LoaderOptions {
   fs: FS;
   extname: string;
   root: string[];
@@ -15,9 +17,13 @@ export enum LookupType {
 }
 export class Loader {
   private options: LoaderOptions
+  private sep: string
+  private rRelativePath: RegExp
 
   constructor (options: LoaderOptions) {
     this.options = options
+    this.sep = this.options.fs.sep || '/'
+    this.rRelativePath = new RegExp(['.' + this.sep, '..' + this.sep].map(prefix => escapeRegex(prefix)).join('|'))
   }
 
   public * lookup (file: string, type: LookupType, sync?: boolean, currentFile?: string) {
@@ -29,20 +35,12 @@ export class Loader {
     throw this.lookupError(file, dirs)
   }
 
-  public shouldLoadRelative (currentFile: string) {
-    return this.options.relativeReference && this.isRelativePath(currentFile)
-  }
-
-  public isRelativePath (path: string) {
-    return path.startsWith('./') || path.startsWith('../')
-  }
-
   public * candidates (file: string, dirs: string[], currentFile?: string, enforceRoot?: boolean) {
     const { fs, extname } = this.options
     if (this.shouldLoadRelative(file) && currentFile) {
-      const referenced = fs.resolve(this.dirname(currentFile), file, extname)
+      const referenced = fs.resolve(this.dirname(currentFile), file, extname, this.options)
       for (const dir of dirs) {
-        if (!enforceRoot || referenced.startsWith(dir)) {
+        if (!enforceRoot || this.withinDir(referenced, dir)) {
           // the relatively referenced file is within one of root dirs
           yield referenced
           break
@@ -51,7 +49,7 @@ export class Loader {
     }
     for (const dir of dirs) {
       const referenced = fs.resolve(dir, file, extname)
-      if (!enforceRoot || referenced.startsWith(dir)) {
+      if (!enforceRoot || this.withinDir(referenced, dir)) {
         yield referenced
       }
     }
@@ -61,10 +59,19 @@ export class Loader {
     }
   }
 
+  private withinDir (file: string, dir: string) {
+    dir = dir.endsWith(this.sep) ? dir : dir + this.sep
+    return file.startsWith(dir)
+  }
+
+  private shouldLoadRelative (referencedFile: string) {
+    return this.options.relativeReference && this.rRelativePath.test(referencedFile)
+  }
+
   private dirname (path: string) {
-    const segments = path.split('/')
-    segments.pop()
-    return segments.join('/')
+    const fs = this.options.fs
+    assert(fs.dirname, '`fs.dirname` is required for relative reference')
+    return fs.dirname!(path)
   }
 
   private lookupError (file: string, roots: string[]) {

src/fs/node.ts

@@ -1,6 +1,7 @@
 import * as _ from '../util/underscore'
-import { resolve as nodeResolve, extname } from 'path'
+import { resolve as nodeResolve, extname, dirname as nodeDirname } from 'path'
 import { stat, statSync, readFile as nodeReadFile, readFileSync as nodeReadFileSync } from 'fs'
+import { LiquidOptions } from '../liquid-options'
 
 const statAsync = _.promisify(stat)
 const readFileAsync = _.promisify<string, string, string>(nodeReadFile)
@@ -27,7 +28,7 @@ export function existsSync (filepath: string) {
 export function readFileSync (filepath: string) {
   return nodeReadFileSync(filepath, 'utf8')
 }
-export function resolve (root: string, file: string, ext: string) {
+export function resolve (root: string, file: string, ext: string, opts: LiquidOptions) {
   if (!extname(file)) file += ext
   return nodeResolve(root, file)
 }
@@ -36,3 +37,7 @@ export function fallback (file: string) {
     return require.resolve(file)
   } catch (e) {}
 }
+export function dirname (filepath: string) {
+  return nodeDirname(filepath)
+}
+export { sep } from 'path'