fix(loader): do not make abs path for pre registered templates

src/Compiler/index.ts

@@ -12,8 +12,8 @@
 */
 
 import { Parser } from 'edge-parser'
-import { IToken, ITagToken } from 'edge-lexer/build/src/Contracts'
-import { ILoader, IPresenterConstructor, ICompiler, Tags } from '../Contracts'
+import { IToken } from 'edge-lexer/build/src/Contracts'
+import { ILoader, ICompiler, Tags, ILoaderTemplate } from '../Contracts'
 import { mergeSections, isBlock } from '../utils'
 import * as Debug from 'debug'
 
@@ -22,12 +22,14 @@ const debug = Debug('edge:loader')
 /**
  * Compiler compiles the template to a function, which can be invoked at a later
  * stage.
+ *
  * Compiler uses [edge-parser](https://npm.im/edge-parser) under the hood and also
  * handles the layouts.
+ *
  * When caching is set to `true`, the compiled templates will be cached to improve performance.
  */
 export class Compiler implements ICompiler {
-  private cacheStore: Map<string, { template: string, Presenter?: IPresenterConstructor }> = new Map()
+  private cacheStore: Map<string, ILoaderTemplate> = new Map()
 
   constructor (private loader: ILoader, private tags: Tags, private cache: boolean = true) {
   }
@@ -37,7 +39,7 @@ export class Compiler implements ICompiler {
    * cache. If caching is disabled, then it will
    * return undefined.
    */
-  private _getFromCache (templatePath: string): undefined | { template: string, Presenter?: IPresenterConstructor } {
+  private _getFromCache (templatePath: string): undefined | ILoaderTemplate {
     if (!this.cache) {
       return
     }
@@ -47,9 +49,9 @@ export class Compiler implements ICompiler {
 
   /**
    * Set's the template path and the payload to the cache. If
-   * cache is disable, then it will never be set.
+   * cache is disabled, then it will never be set.
    */
-  private _setInCache (templatePath: string, payload: { template: string, Presenter?: IPresenterConstructor }) {
+  private _setInCache (templatePath: string, payload: ILoaderTemplate) {
     if (!this.cache) {
       return
     }
@@ -66,7 +68,7 @@ export class Compiler implements ICompiler {
   private _templateContentToTokens (content: string, parser: Parser): IToken[] {
     let templateTokens = parser.generateTokens(content)
 
-    const firstToken = templateTokens[0] as ITagToken
+    const firstToken = templateTokens[0]
 
     if (isBlock(firstToken, 'layout')) {
       debug('detected layout %s', firstToken.properties.jsArg)
@@ -97,7 +99,7 @@ export class Compiler implements ICompiler {
    * later.
    *
    * When `inline` is set to true, the compiled output will not have it's own scope and
-   * neither an attempt to load is made. The `inline` is mainly used for partials.
+   * neither an attempt to load the presenter is made. The `inline` is mainly used for partials.
    *
    * ```js
    * compiler.compile('welcome', false)
@@ -113,7 +115,7 @@ export class Compiler implements ICompiler {
    * }
    * ```
    */
-  public compile (templatePath: string, inline: boolean): { template: string, Presenter?: IPresenterConstructor } {
+  public compile (templatePath: string, inline: boolean): ILoaderTemplate {
     templatePath = this.loader.makePath(templatePath)
 
     /**

src/Context/index.ts

@@ -20,20 +20,14 @@ import { IPresenter } from '../Contracts'
  * Context is used at runtime to resolve values for a given
  * template.
  *
- * Also the context can be extended by tags to add `getters` and `methods`.
+ * Also the context can be extended to add `getters` and `methods`. Checkout
+ * [macroable](https://github.com/poppinss/macroable) for same.
  */
 export class Context extends Macroable {
   protected static _macros = {}
 
   protected static _getters = {}
 
-  /**
-   * Remove all macros and getters
-   */
-  public static hydrate () {
-    super.hydrate()
-  }
-
   /**
    * Frames are used to define a inner scope in which values will
    * be resolved. The resolve function starts with the deepest
@@ -119,7 +113,8 @@ export class Context extends Macroable {
    * 3. Then the presenter `state` object.
    * 4. Finally fallback to the sharedState.
    *
-   * ```
+   * @example
+   * ```js
    * ctx.resolve('username')
    * ```
    */

src/Contracts/index.ts

@@ -2,15 +2,30 @@
  * @module main
  */
 
+/**
+ * edge
+ *
+ * (c) Harminder Virk <virk@adonisjs.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
 import { ITag as BaseTag } from 'edge-parser/build/src/Contracts'
 import { IToken } from 'edge-lexer/build/src/Contracts'
 
+export type ILoaderTemplate = {
+  template: string,
+  Presenter?: IPresenterConstructor,
+}
+
 export interface ILoaderConstructor {
   new (): ILoader
 }
 
 export interface ILoader {
-  mounted: object
+  mounted: { [key: string]: string }
+
   /**
    * Save disk name and dirPath to resolve views
    */
@@ -24,7 +39,7 @@ export interface ILoader {
   /**
    * Resolve template contents and optionally the Presenter
    */
-  resolve (templatePath: string, withPresenter: boolean): { template: string, Presenter?: IPresenterConstructor }
+  resolve (templatePath: string, withPresenter: boolean): ILoaderTemplate
 
   /**
    * Make absolute path to a template
@@ -34,7 +49,7 @@ export interface ILoader {
   /**
    * Register in memory template and presenter
    */
-  register (templatePath: string, contents: { template: string, Presenter?: IPresenterConstructor }): void
+  register (templatePath: string, contents: ILoaderTemplate): void
 }
 
 export interface ICompiler {
@@ -46,7 +61,7 @@ export interface ICompiler {
   /**
    * Compile template to a function string
    */
-  compile (templatePath: string, inline: boolean): { template: string, Presenter?: IPresenterConstructor }
+  compile (templatePath: string, inline: boolean): ILoaderTemplate
 }
 
 export interface ITag extends BaseTag {

src/Loader/index.ts

@@ -2,42 +2,42 @@
  * @module main
  */
 
-/*
-* edge
-*
-* (c) Harminder Virk <virk@adonisjs.com>
-*
-* For the full copyright and license information, please view the LICENSE
-* file that was distributed with this source code.
-*/
+/**
+ * edge
+ *
+ * (c) Harminder Virk <virk@adonisjs.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
 
 import { join, isAbsolute, extname } from 'path'
 import { readFileSync } from 'fs'
 import * as requireUncached from 'require-uncached'
 
-import { ILoader, IPresenterConstructor } from '../Contracts'
+import { ILoader, IPresenterConstructor, ILoaderTemplate } from '../Contracts'
 import { extractDiskAndTemplateName } from '../utils'
 import * as Debug from 'debug'
 
 const debug = Debug('edge:loader')
 
 /**
  * The job of a loader is to load the template and it's presenter for a given path.
- * The base loader (shipped with edge) looks for files on the file-system.
+ * The base loader (shipped with edge) looks for files on the file-system and
+ * reads them synchronously.
  *
- * However, you are free to add your own loader implementation. Just make sure it implements
- * the ILoader interface.
+ * You are free to define your own loaders that implements the [[ILoader]] interface
  */
 export class Loader implements ILoader {
   private mountedDirs: Map<string, string> = new Map()
-  private preRegistered: Map<string, { template: string, Presenter?: IPresenterConstructor }> = new Map()
+  private preRegistered: Map<string, ILoaderTemplate> = new Map()
 
   /**
    * Attempts to load the presenter for a given template. If presenter doesn't exists, it
    * will swallow the error.
    *
-   * Also this method will **bypass the require cache**, since in product compiled templates and their
-   * presenters are cached anyways.
+   * Also this method will **bypass the require cache**, since in production compiled templates
+   * and their presenters are cached anyways.
    */
   private _getPresenterForTemplate (templatePath: string): IPresenterConstructor | undefined {
     try {
@@ -46,6 +46,7 @@ export class Loader implements ILoader {
         .replace(extname(templatePath), '.presenter.js')
 
       debug('loading presenter %s', presenterPath)
+
       return requireUncached(presenterPath)
     } catch (error) {
       if (['ENOENT', 'MODULE_NOT_FOUND'].indexOf(error.code) === -1) {
@@ -68,7 +69,7 @@ export class Loader implements ILoader {
    * }
    * ```
    */
-  public get mounted (): object {
+  public get mounted (): { [key: string]: string } {
     return Array.from(this.mountedDirs).reduce((obj, [key, value]) => {
       obj[key] = value
       return obj
@@ -117,6 +118,10 @@ export class Loader implements ILoader {
    * @throws Error if disk is not mounted and attempting to make path for it.
    */
   public makePath (templatePath: string): string {
+    if (this.preRegistered.has(templatePath)) {
+      return templatePath
+    }
+
     const [diskName, template] = extractDiskAndTemplateName(templatePath)
 
     const mountedDir = this.mountedDirs.get(diskName)
@@ -128,8 +133,9 @@ export class Loader implements ILoader {
   }
 
   /**
-   * Resolves the template for the disk optionally loads the presenter too. The presenter
-   * resolution is based on the convention.
+   * Resolves the template from the disk, optionally loads the presenter too. The presenter
+   * resolution is based on the convention and resolved from the same directory
+   * as the template.
    *
    * ## Presenter convention
    * - View name - welcome
@@ -145,29 +151,25 @@ export class Loader implements ILoader {
    * }
    * ```
    */
-  public resolve (templatePath: string, withPresenter: boolean): {
-    template: string,
-    Presenter?: IPresenterConstructor,
-  } {
+  public resolve (templatePath: string, withPresenter: boolean): ILoaderTemplate {
     debug('attempting to resolve %s', templatePath)
-    debug('with presenter %s', withPresenter)
+    debug('with presenter %s', String(withPresenter))
+
+    /**
+     * Return from pre-registered one's if exists
+     */
+    if (this.preRegistered.has(templatePath)) {
+      const contents = this.preRegistered.get(templatePath)
+      return withPresenter ? contents! : { template: contents!.template }
+    }
 
     try {
       templatePath = isAbsolute(templatePath) ? templatePath : this.makePath(templatePath)
 
-      /**
-       * Return from pre-registered one's if exists
-       */
-      if (this.preRegistered.get(templatePath)) {
-        const contents = this.preRegistered.get(templatePath)
-        return withPresenter ? contents! : { template: contents!.template }
-      }
-
       const template = readFileSync(templatePath, 'utf-8')
       const presenter = withPresenter ? this._getPresenterForTemplate(templatePath) : undefined
 
       debug('has presenter %s', !!presenter)
-
       return { template, Presenter: presenter }
     } catch (error) {
       if (error.code === 'ENOENT') {
@@ -195,12 +197,15 @@ export class Loader implements ILoader {
    *
    * @throws Error if template content is empty.
    */
-  public register (templatePath: string, contents: { template: string, Presenter?: IPresenterConstructor }) {
+  public register (templatePath: string, contents: ILoaderTemplate) {
     if (!contents.template) {
       throw new Error('Make sure to define the template content for preRegistered template')
     }
 
-    templatePath = isAbsolute(templatePath) ? templatePath : this.makePath(templatePath)
+    if (this.preRegistered.has(templatePath)) {
+      throw new Error(`Cannot override previously registered ${templatePath} template`)
+    }
+
     this.preRegistered.set(templatePath, contents)
   }
 }

test/compiler.spec.ts

@@ -18,7 +18,6 @@ const tags = {
   if: class If {
     public static block = true
     public static seekable = true
-    public static selfclosed = false
     public static tagName = 'if'
     public static compile (): void {
     }

test/edge.spec.ts

@@ -18,7 +18,7 @@ import { Compiler } from '../src/Compiler'
 
 const viewsDir = join(__dirname, 'views')
 
-test.group('Template', (group) => {
+test.group('Edge', (group) => {
   group.afterEach(async () => {
     Edge.clear()
     await fs.remove(viewsDir)
@@ -86,7 +86,8 @@ test.group('Template', (group) => {
   })
 
   test('register a template as a string', async (assert) => {
-    Edge.mount(viewsDir)
+    Edge.configure({})
+
     Edge.register('foo', {
       template: `Hello {{ username }}`,
     })

test/loader.spec.ts

@@ -34,13 +34,13 @@ test.group('Loader', (group) => {
     assert.deepEqual(loader.mounted, {})
   })
 
-  test('throw exception when resolve path from undefined location', (assert) => {
+  test('throw exception when resolving path from undefined location', (assert) => {
     const loader = new Loader()
     const fn = () => loader.resolve('foo', true)
     assert.throw(fn, 'Attempting to resolve foo.edge template for unmounted default location')
   })
 
-  test('resolve template for given location', async (assert) => {
+  test('resolve template for default location', async (assert) => {
     await fs.outputFile(join(viewsDir, 'foo.edge'), 'Hello world')
 
     const loader = new Loader()
@@ -135,4 +135,23 @@ test.group('Loader', (group) => {
     assert.equal(template.trim(), 'Hello world')
     assert.isUndefined(Presenter)
   })
+
+  test('pre register templates with a key', async (assert) => {
+    const loader = new Loader()
+    loader.register('my-view', {
+      template: 'Hello world',
+    })
+
+    const { template, Presenter } = loader.resolve('my-view', true)
+    assert.equal(template.trim(), 'Hello world')
+    assert.isUndefined(Presenter)
+  })
+
+  test('pre registering duplicate templates must raise an error', async (assert) => {
+    const loader = new Loader()
+    loader.register('my-view', { template: 'Hello world' })
+    const fn = () => loader.register('my-view', { template: 'Hello world' })
+
+    assert.throw(fn, 'Cannot override previously registered my-view template')
+  })
 })