fix: relative root (by default) yields LookupError, fixes #419, #424,…

… also related to #395

src/fs/fs.ts

@@ -9,9 +9,11 @@ export interface FS {
   readFileSync: (filepath: string) => string;
   /** resolve a file against directory, for given `ext` option */
   resolve: (dir: string, file: string, ext: string) => string;
-  /** defaults to "/", will be used for "within roots" check */
+  /** check if file is contained in `root`, always return `true` by default */
+  contains?: (root: string, file: string) => boolean;
+  /** defaults to "/" */
   sep?: string;
-  /** dirname for a filepath, used when resolving relative path */
+  /** required for relative path resolving */
   dirname?: (file: string) => string;
   /** fallback file for lookup failure */
   fallback?: (file: string) => string | undefined;

src/fs/loader.ts

@@ -16,14 +16,21 @@ export enum LookupType {
   Root = 'root'
 }
 export class Loader {
+  public shouldLoadRelative: (referencedFile: string) => boolean
   private options: LoaderOptions
-  private sep: string
-  private rRelativePath: RegExp
+  private contains: (root: string, file: string) => boolean
 
   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('|'))
+    if (options.relativeReference) {
+      const sep = options.fs.sep
+      assert(sep, '`fs.sep` is required for relative reference')
+      const rRelativePath = new RegExp(['.' + sep, '..' + sep].map(prefix => escapeRegex(prefix)).join('|'))
+      this.shouldLoadRelative = (referencedFile: string) => rRelativePath.test(referencedFile)
+    } else {
+      this.shouldLoadRelative = (referencedFile: string) => false
+    }
+    this.contains = this.options.fs.contains || (() => true)
   }
 
   public * lookup (file: string, type: LookupType, sync?: boolean, currentFile?: string) {
@@ -35,16 +42,12 @@ export class Loader {
     throw this.lookupError(file, dirs)
   }
 
-  public shouldLoadRelative (referencedFile: string) {
-    return this.options.relativeReference && this.rRelativePath.test(referencedFile)
-  }
-
   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)
       for (const dir of dirs) {
-        if (!enforceRoot || this.withinDir(referenced, dir)) {
+        if (!enforceRoot || this.contains(dir, referenced)) {
           // the relatively referenced file is within one of root dirs
           yield referenced
           break
@@ -53,7 +56,7 @@ export class Loader {
     }
     for (const dir of dirs) {
       const referenced = fs.resolve(dir, file, extname)
-      if (!enforceRoot || this.withinDir(referenced, dir)) {
+      if (!enforceRoot || this.contains(dir, referenced)) {
         yield referenced
       }
     }
@@ -63,11 +66,6 @@ export class Loader {
     }
   }
 
-  private withinDir (file: string, dir: string) {
-    dir = dir.endsWith(this.sep) ? dir : dir + this.sep
-    return file.startsWith(dir)
-  }
-
   private dirname (path: string) {
     const fs = this.options.fs
     assert(fs.dirname, '`fs.dirname` is required for relative reference')

src/fs/node.ts

@@ -1,5 +1,5 @@
 import * as _ from '../util/underscore'
-import { resolve as nodeResolve, extname, dirname as nodeDirname } from 'path'
+import { sep, resolve as nodeResolve, extname, dirname as nodeDirname } from 'path'
 import { stat, statSync, readFile as nodeReadFile, readFileSync as nodeReadFileSync } from 'fs'
 
 const statAsync = _.promisify(stat)
@@ -39,4 +39,10 @@ export function fallback (file: string) {
 export function dirname (filepath: string) {
   return nodeDirname(filepath)
 }
+export function contains (root: string, file: string) {
+  root = nodeResolve(root)
+  root = root.endsWith(sep) ? root : root + sep
+  return file.startsWith(root)
+}
+
 export { sep } from 'path'

src/liquid-options.ts

@@ -131,16 +131,13 @@ export const defaultOptions: NormalizedFullOptions = {
   operatorsTrie: createTrie(defaultOperators)
 }
 
-export function normalize (options?: LiquidOptions): NormalizedOptions {
-  options = options || {}
-  if (options.hasOwnProperty('root')) {
-    options.root = normalizeDirectoryList(options.root)
-  }
-  if (options.hasOwnProperty('partials')) {
-    options.partials = normalizeDirectoryList(options.partials)
+export function normalize (options: LiquidOptions): NormalizedFullOptions {
+  if (options.hasOwnProperty('operators')) {
+    (options as NormalizedOptions).operatorsTrie = createTrie(options.operators!)
   }
-  if (options.hasOwnProperty('layouts')) {
-    options.layouts = normalizeDirectoryList(options.layouts)
+  if (options.hasOwnProperty('root')) {
+    if (!options.hasOwnProperty('partials')) options.partials = options.root
+    if (!options.hasOwnProperty('layouts')) options.layouts = options.root
   }
   if (options.hasOwnProperty('cache')) {
     let cache: Cache<Thenable<Template[]>> | undefined
@@ -149,21 +146,15 @@ export function normalize (options?: LiquidOptions): NormalizedOptions {
     else cache = options.cache ? new LRU(1024) : undefined
     options.cache = cache
   }
-  if (options.hasOwnProperty('operators')) {
-    (options as NormalizedOptions).operatorsTrie = createTrie(options.operators!)
-  }
-  return options as NormalizedOptions
-}
-
-export function applyDefault (options: NormalizedOptions): NormalizedFullOptions {
-  const fullOptions = { ...defaultOptions, ...options }
-  if (fullOptions.partials === defaultOptions.partials) {
-    fullOptions.partials = fullOptions.root
-  }
-  if (fullOptions.layouts === defaultOptions.layouts) {
-    fullOptions.layouts = fullOptions.root
+  options = { ...defaultOptions, ...options }
+  if (!options.fs!.dirname && options.relativeReference) {
+    console.warn('[LiquidJS] `fs.dirname` is required for relativeReference, set relativeReference to `false` to suppress this warning, or provide implementation for `fs.dirname`')
+    options.relativeReference = false
   }
-  return fullOptions
+  options.root = normalizeDirectoryList(options.root)
+  options.partials = normalizeDirectoryList(options.partials)
+  options.layouts = normalizeDirectoryList(options.layouts)
+  return options as NormalizedFullOptions
 }
 
 export function normalizeDirectoryList (value: any): string[] {

src/liquid.ts

@@ -10,7 +10,7 @@ import builtinTags from './builtin/tags'
 import * as builtinFilters from './builtin/filters'
 import { TagMap } from './template/tag/tag-map'
 import { FilterMap } from './template/filter/filter-map'
-import { LiquidOptions, normalizeDirectoryList, NormalizedFullOptions, applyDefault, normalize } from './liquid-options'
+import { LiquidOptions, normalizeDirectoryList, NormalizedFullOptions, normalize } from './liquid-options'
 import { FilterImplOptions } from './template/filter/filter-impl-options'
 import { toPromise, toValue } from './util/async'
 
@@ -26,7 +26,7 @@ export class Liquid {
   public readonly tags: TagMap
 
   public constructor (opts: LiquidOptions = {}) {
-    this.options = applyDefault(normalize(opts))
+    this.options = normalize(opts)
     this.parser = new Parser(this)
     this.renderer = new Render()
     this.filters = new FilterMap(this.options.strictFilters, this)
@@ -117,7 +117,10 @@ export class Liquid {
     return function (this: any, filePath: string, ctx: object, callback: (err: Error | null, rendered: string) => void) {
       if (firstCall) {
         firstCall = false
-        self.options.root.unshift(...normalizeDirectoryList(this.root))
+        const dirs = normalizeDirectoryList(this.root)
+        self.options.root.unshift(...dirs)
+        self.options.layouts.unshift(...dirs)
+        self.options.partials.unshift(...dirs)
       }
       self.renderFile(filePath, ctx).then(html => callback(null, html) as any, callback as any)
     }

test/integration/builtin/tags/layout.ts

@@ -85,6 +85,17 @@ describe('tags/layout', function () {
     const html = await liquid.parseAndRender(src)
     return expect(html).to.equal('XAY')
   })
+  it('should use `layouts` if specified', async function () {
+    mock({
+      '/layouts/parent.html': 'LAYOUTS {%block%}{%endblock%}',
+      '/root/parent.html': 'ROOT {%block%}{%endblock%}',
+      '/root/main.html': '{% layout parent.html %}{%block%}A{%endblock%}'
+    })
+    const staticLiquid = new Liquid({ root: '/root', layouts: '/layouts', dynamicPartials: false })
+    const html = await staticLiquid.renderFile('main.html')
+    return expect(html).to.equal('LAYOUTS A')
+  })
+
   it('should support block.super', async function () {
     mock({
       '/parent.html': '{% block css %}<link href="base.css" rel="stylesheet">{% endblock %}'
@@ -189,6 +200,16 @@ describe('tags/layout', function () {
     return expect(html).to.equal('blackA')
   })
 
+  it('should support relative root', async function () {
+    mock({
+      [process.cwd() + '/foo/parent.html']: '{{color}}{%block%}{%endblock%}',
+      [process.cwd() + '/foo/bar/main.html']: '{% layout parent.html color:"black"%}{%block%}A{%endblock%}'
+    })
+    const staticLiquid = new Liquid({ root: './foo', dynamicPartials: false })
+    const html = await staticLiquid.renderFile('bar/main.html')
+    return expect(html).to.equal('blackA')
+  })
+
   describe('static partial', function () {
     it('should support filename with extension', async function () {
       mock({

test/unit/liquid-options.ts

@@ -3,13 +3,9 @@ import { expect } from 'chai'
 
 describe('liquid-options', () => {
   describe('.normalize()', () => {
-    it('should return plain object for empty input', () => {
-      const options = normalize()
-      expect(JSON.stringify(options)).to.equal('{}')
-    })
-    it('should set falsy cache to undefined', () => {
+    it('should set cache to undefined if specified to falsy', () => {
       const options = normalize({ cache: false })
-      expect(JSON.stringify(options)).to.equal('{}')
+      expect(options.cache).to.equal(undefined)
     })
   })
 })