perf: parse filenames in parse() insteadof render()

src/builtin/tags/include.ts

@@ -1,14 +1,14 @@
-import { assert, evalQuotedToken, TypeGuards, Tokenizer, evalToken, Hash, Emitter, TagToken, Context, TagImplOptions } from '../../types'
+import { assert, Tokenizer, evalToken, Hash, Emitter, TagToken, Context, TagImplOptions } from '../../types'
 import BlockMode from '../../context/block-mode'
+import { parseFilePath, renderFilePath } from './render'
 
 export default {
+  parseFilePath,
+  renderFilePath,
   parse: function (token: TagToken) {
     const args = token.args
     const tokenizer = new Tokenizer(args, this.liquid.options.operatorsTrie)
-    this.file = this.liquid.options.dynamicPartials
-      ? tokenizer.readValue()
-      : tokenizer.readFileName()
-    assert(this.file, () => `illegal argument "${token.args}"`)
+    this['file'] = this.parseFilePath(tokenizer, this.liquid)
 
     const begin = tokenizer.p
     const withStr = tokenizer.readIdentifier()
@@ -22,15 +22,10 @@ export default {
     this.hash = new Hash(tokenizer.remaining())
   },
   render: function * (ctx: Context, emitter: Emitter) {
-    const { liquid, hash, withVar, file } = this
+    const { liquid, hash, withVar } = this
     const { renderer } = liquid
-    // TODO try move all liquid.parse calls into parse() section
-    const filepath = ctx.opts.dynamicPartials
-      ? (TypeGuards.isQuotedToken(file)
-        ? yield renderer.renderTemplates(liquid.parse(evalQuotedToken(file)), ctx)
-        : yield evalToken(file, ctx))
-      : file.getText()
-    assert(filepath, () => `illegal filename "${file.getText()}":"${filepath}"`)
+    const filepath = yield this.renderFilePath(this['file'], ctx, liquid)
+    assert(filepath, () => `illegal filename "${filepath}"`)
 
     const saved = ctx.saveRegister('blocks', 'blockMode')
     ctx.setRegister('blocks', {})

src/builtin/tags/layout.ts

@@ -1,31 +1,27 @@
-import { assert, evalQuotedToken, TypeGuards, evalToken, Tokenizer, Emitter, Hash, TagToken, TopLevelToken, Context, TagImplOptions } from '../../types'
+import { assert, Tokenizer, Emitter, Hash, TagToken, TopLevelToken, Context, TagImplOptions } from '../../types'
 import BlockMode from '../../context/block-mode'
+import { parseFilePath, renderFilePath } from './render'
 
 export default {
+  parseFilePath,
+  renderFilePath,
   parse: function (token: TagToken, remainTokens: TopLevelToken[]) {
     const tokenizer = new Tokenizer(token.args, this.liquid.options.operatorsTrie)
-    const file = this.liquid.options.dynamicPartials ? tokenizer.readValue() : tokenizer.readFileName()
-    assert(file, () => `illegal argument "${token.args}"`)
-
-    this.file = file
+    this['file'] = this.parseFilePath(tokenizer, this.liquid)
     this.hash = new Hash(tokenizer.remaining())
     this.tpls = this.liquid.parser.parse(remainTokens)
   },
   render: function * (ctx: Context, emitter: Emitter) {
     const { liquid, hash, file } = this
     const { renderer } = liquid
-    if (file.getText() === 'none') {
+    if (file === null) {
       ctx.setRegister('blockMode', BlockMode.OUTPUT)
       const html = yield renderer.renderTemplates(this.tpls, ctx)
       emitter.write(html)
       return
     }
-    const filepath = ctx.opts.dynamicPartials
-      ? (TypeGuards.isQuotedToken(file)
-        ? yield renderer.renderTemplates(liquid.parse(evalQuotedToken(file)), ctx)
-        : evalToken(this.file, ctx))
-      : file.getText()
-    assert(filepath, () => `file "${file.getText()}"("${filepath}") not available`)
+    const filepath = yield this.renderFilePath(this['file'], ctx, liquid)
+    assert(filepath, () => `illegal filename "${filepath}"`)
     const templates = yield liquid.parseFileImpl(filepath, ctx.sync)
 
     // render remaining contents and store rendered results

src/builtin/tags/render.ts

@@ -1,16 +1,16 @@
 import { assert } from '../../util/assert'
 import { ForloopDrop } from '../../drop/forloop-drop'
 import { toEnumerable } from '../../util/collection'
-import { evalQuotedToken, TypeGuards, Tokenizer, evalToken, Hash, Emitter, TagToken, Context, TagImplOptions } from '../../types'
+import { Liquid } from '../../liquid'
+import { Token, Template, evalQuotedToken, TypeGuards, Tokenizer, evalToken, Hash, Emitter, TagToken, Context, TagImplOptions } from '../../types'
 
 export default {
+  parseFilePath,
+  renderFilePath,
   parse: function (token: TagToken) {
     const args = token.args
     const tokenizer = new Tokenizer(args, this.liquid.options.operatorsTrie)
-    this.file = this.liquid.options.dynamicPartials
-      ? tokenizer.readValue()
-      : tokenizer.readFileName()
-    assert(this.file, () => `illegal argument "${token.args}"`)
+    this['file'] = this.parseFilePath(tokenizer, this.liquid)
 
     while (!tokenizer.end()) {
       tokenizer.skipBlank()
@@ -40,14 +40,9 @@ export default {
     this.hash = new Hash(tokenizer.remaining())
   },
   render: function * (ctx: Context, emitter: Emitter) {
-    const { liquid, file, hash } = this
-    const { renderer } = liquid
-    const filepath = ctx.opts.dynamicPartials
-      ? (TypeGuards.isQuotedToken(file)
-        ? yield renderer.renderTemplates(liquid.parse(evalQuotedToken(file)), ctx)
-        : evalToken(file, ctx))
-      : file.getText()
-    assert(filepath, () => `illegal filename "${file.getText()}":"${filepath}"`)
+    const { liquid, hash } = this
+    const filepath = yield this.renderFilePath(this['file'], ctx, liquid)
+    assert(filepath, () => `illegal filename "${filepath}"`)
 
     const childCtx = new Context({}, ctx.opts, ctx.sync)
     const scope = yield hash.render(ctx)
@@ -65,12 +60,47 @@ export default {
       for (const item of collection) {
         scope[alias] = item
         const templates = yield liquid.parseFileImpl(filepath, childCtx.sync)
-        yield renderer.renderTemplates(templates, childCtx, emitter)
+        yield liquid.renderer.renderTemplates(templates, childCtx, emitter)
         scope.forloop.next()
       }
     } else {
       const templates = yield liquid.parseFileImpl(filepath, childCtx.sync)
-      yield renderer.renderTemplates(templates, childCtx, emitter)
+      yield liquid.renderer.renderTemplates(templates, childCtx, emitter)
     }
   }
 } as TagImplOptions
+
+type ParsedFileName = Template[] | Token | string | undefined
+
+/**
+ * @return null for "none",
+ * @return Template[] for quoted with tags and/or filters
+ * @return Token for expression (not quoted)
+ * @throws TypeError if cannot read next token
+ */
+export function parseFilePath (tokenizer: Tokenizer, liquid: Liquid): ParsedFileName | null {
+  if (liquid.options.dynamicPartials) {
+    const file = tokenizer.readValue()
+    if (file === undefined) throw new TypeError(`illegal argument "${tokenizer.input}"`)
+    if (file.getText() === 'none') return null
+    // for filenames like "files/{{file}}", eval as liquid template
+    if (TypeGuards.isQuotedToken(file)) {
+      const tpls = liquid.parse(evalQuotedToken(file))
+      // for filenames like "files/file.liquid", extract the string directly
+      if (tpls.length === 1) {
+        const first = tpls[0]
+        if (TypeGuards.isHTMLToken(first)) return first.getText()
+      }
+      return tpls
+    }
+    return file
+  }
+  const filepath = tokenizer.readFileName().getText()
+  return filepath === 'none' ? null : filepath
+}
+
+export function renderFilePath (file: ParsedFileName, ctx: Context, liquid: Liquid) {
+  if (typeof file === 'string') return file
+  if (Array.isArray(file)) return liquid.renderer.renderTemplates(file, ctx)
+  return evalToken(file, ctx)
+}

src/parser/tokenizer.ts

@@ -31,7 +31,7 @@ export class Tokenizer {
   private rawBeginAt = -1
 
   constructor (
-    private input: string,
+    public input: string,
     private trie: Trie,
     private file: string = ''
   ) {

src/render/emitter.ts

@@ -14,7 +14,7 @@ export class Emitter {
     if (this.keepOutputType === true) {
       html = toValue(html)
     } else {
-      html = stringify(toValue(html))
+      html = stringify(html)
     }
     // This will only preserve the type if the value is isolated.
     // I.E:

src/render/expression.ts

@@ -47,10 +47,9 @@ export function evalToken (token: Token | undefined, ctx: Context, lenient = fal
 }
 
 function evalPropertyAccessToken (token: PropertyAccessToken, ctx: Context, lenient: boolean) {
-  const variable = token.getVariableAsText()
   const props: string[] = token.props.map(prop => evalToken(prop, ctx, false))
   try {
-    return ctx.get([variable, ...props])
+    return ctx.get([token.propertyName, ...props])
   } catch (e) {
     if (lenient && e.name === 'InternalUndefinedVariableError') return null
     throw (new UndefinedVariableError(e, token))

src/tokens/property-access-token.ts

@@ -5,19 +5,15 @@ import { TokenKind } from '../parser/token-kind'
 import { parseStringLiteral } from '../parser/parse-string-literal'
 
 export class PropertyAccessToken extends Token {
+  public propertyName: string
   constructor (
     public variable: IdentifierToken | QuotedToken,
     public props: (IdentifierToken | QuotedToken | PropertyAccessToken)[],
     end: number
   ) {
     super(TokenKind.PropertyAccess, variable.input, variable.begin, end, variable.file)
-  }
-
-  getVariableAsText () {
-    if (this.variable instanceof IdentifierToken) {
-      return this.variable.getText()
-    } else {
-      return parseStringLiteral(this.variable.getText())
-    }
+    this.propertyName = this.variable instanceof IdentifierToken
+      ? this.variable.getText()
+      : parseStringLiteral(this.variable.getText())
   }
 }

src/util/underscore.ts

@@ -3,13 +3,8 @@ import { Drop } from '../drop/drop'
 const toStr = Object.prototype.toString
 const toLowerCase = String.prototype.toLowerCase
 
-/*
- * Checks if value is classified as a String primitive or object.
- * @param {any} value The value to check.
- * @return {Boolean} Returns true if value is a string, else false.
- */
 export function isString (value: any): value is string {
-  return toStr.call(value) === '[object String]'
+  return typeof value === 'string'
 }
 
 export function isFunction (value: any): value is Function {
@@ -30,7 +25,9 @@ export function promisify (fn: any) {
 
 export function stringify (value: any): string {
   value = toValue(value)
-  return isNil(value) ? '' : String(value)
+  if (isString(value)) return value
+  if (isNil(value)) return ''
+  return String(value)
 }
 
 export function toValue (value: any): any {
@@ -47,7 +44,7 @@ export function toLiquid (value: any): any {
 }
 
 export function isNil (value: any): boolean {
-  return value === null || value === undefined
+  return value == null
 }
 
 export function isArray (value: any): value is any[] {

test/integration/builtin/tags/include.ts

@@ -44,7 +44,7 @@ describe('tags/include', function () {
     })
     return liquid.renderFile('/parent.html').catch(function (e) {
       expect(e.name).to.equal('RenderError')
-      expect(e.message).to.match(/illegal filename "not-exist"/)
+      expect(e.message).to.match(/illegal filename "undefined"/)
     })
   })
 

test/integration/builtin/tags/layout.ts

@@ -38,7 +38,7 @@ describe('tags/layout', function () {
     })
     return liquid.renderFile('/parent.html').catch(function (e) {
       expect(e.name).to.equal('RenderError')
-      expect(e.message).to.contain('file "foo"("undefined") not available')
+      expect(e.message).to.contain('illegal filename "undefined"')
     })
   })
   it('should handle layout none', async function () {

test/integration/builtin/tags/render.ts

@@ -44,7 +44,7 @@ describe('tags/render', function () {
     })
     return liquid.renderFile('/parent.html').catch(function (e) {
       expect(e.name).to.equal('RenderError')
-      expect(e.message).to.match(/illegal filename "not-exist":"undefined"/)
+      expect(e.message).to.match(/illegal filename "undefined"/)
     })
   })
 

test/integration/util/error.ts

@@ -269,9 +269,8 @@ describe('error', function () {
         .to.throw(RenderError, /intended render error/)
     })
     it('should contain original error info for {% include %}', function () {
-      const origin = ['1st', '2nd', '3rd', 'X{%throwingTag%} Y', '5th', '6th', '7th']
       mock({
-        '/throwing-tag.html': origin.join('\n')
+        '/throwing-tag.html': ['1st', '2nd', '3rd', 'X{%throwingTag%} Y', '5th', '6th', '7th'].join('\n')
       })
       const html = '{%include "throwing-tag.html"%}'
       const message = [

test/unit/tokens/property-access-token.ts

@@ -8,14 +8,14 @@ chai.use(sinonChai)
 const expect = chai.expect
 
 describe('PropertyAccessToken', function () {
-  describe('getVariableAsText', function () {
+  describe('#propertyName', function () {
     it('should return correct value for IdentifierToken', function () {
       const token = new PropertyAccessToken(new IdentifierToken('foo', 0, 3), [], 3)
-      expect(token.getVariableAsText()).to.equal('foo')
+      expect(token.propertyName).to.equal('foo')
     })
     it('should return correct value for QuotedToken', function () {
       const token = new PropertyAccessToken(new QuotedToken('"foo bar"', 0, 9), [], 9)
-      expect(token.getVariableAsText()).to.equal('foo bar')
+      expect(token.propertyName).to.equal('foo bar')
     })
   })
 })