From 211bb7b3da9d9ddc4f055d871255d8bdfabac888 Mon Sep 17 00:00:00 2001
From: Eric Hwang <e@lever.co>
Date: Thu, 5 Oct 2023 15:59:07 -0700
Subject: [PATCH] Code review cleanups

---
 src/Page.ts                  |  4 +---
 src/parsing/index.ts         | 13 ++++++++++---
 src/templates/expressions.ts | 15 +++------------
 src/templates/templates.ts   |  4 ++--
 4 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/Page.ts b/src/Page.ts
index df77e363..ec998fac 100644
--- a/src/Page.ts
+++ b/src/Page.ts
@@ -68,9 +68,7 @@ export abstract class PageBase extends Controller {
 
   destroy() {
     this.emit('destroy');
-    if (this._removeModelListeners) {
-      this._removeModelListeners();
-    }
+    this._removeModelListeners();
     for (const id in this._components) {
       const component = this._components[id];
       component.destroy();
diff --git a/src/parsing/index.ts b/src/parsing/index.ts
index eb7e3a33..663d1ccd 100644
--- a/src/parsing/index.ts
+++ b/src/parsing/index.ts
@@ -4,7 +4,7 @@ import htmlUtil = require('html-util');
 
 import { createPathExpression } from './createPathExpression';
 import { markup } from './markup';
-import { App } from '../App';
+import { App, AppBase } from '../App';
 import { templates, expressions } from '../templates';
 import { Expression } from '../templates/expressions';
 import { MarkupHook, View } from '../templates/templates';
@@ -18,6 +18,13 @@ declare module '../App' {
   }
 }
 
+interface ParsedView {
+  name: string;
+  source: string;
+  options: unknown;
+  filename?: string;
+}
+
 // View.prototype._parse is defined here, so that it doesn't have to
 // be included in the client if templates are all parsed server-side
 templates.View.prototype._parse = function() {
@@ -881,7 +888,7 @@ export function getImportNamespace(namespace: string, attrs: Record<string, stri
 }
 
 export function parseViews(file: string, namespace: string, filename?: string, onImport?: (attrs) => void) {
-  const views = [];
+  const views: ParsedView[] = [];
   const prefix = (namespace) ? namespace + ':' : '';
 
   htmlUtil.parse(file + '\n', {
@@ -933,7 +940,7 @@ export function parseViews(file: string, namespace: string, filename?: string, o
   return views;
 }
 
-export function registerParsedViews(app, items) {
+export function registerParsedViews(app: AppBase, items: ParsedView[]) {
   for (let i = 0, len = items.length; i < len; i++) {
     const item = items[i];
     app.views.register(item.name, item.source, item.options);
diff --git a/src/templates/expressions.ts b/src/templates/expressions.ts
index c006c321..e0ca9ae5 100644
--- a/src/templates/expressions.ts
+++ b/src/templates/expressions.ts
@@ -6,17 +6,11 @@ import * as operatorFns from './operatorFns';
 import { ContextClosure, Dependency, Template } from './templates';
 import { concat } from './util';
 import { Component } from '../components';
-import { Controller } from '../Controller';
-import { Page } from '../Page';
 
 type SegmentOrContext = string | number | { item: number } | Context;
 type Segment = string | number;
 type Value = any; // global | Page | ModelData
 
-function isPage(controller: Controller): controller is Page {
-  return !Object.prototype.hasOwnProperty.call((controller as Page), '_scope');
-}
-
 export function lookup(segments: Segment[] | undefined, value: Value) {
   if (!segments) return value;
 
@@ -685,12 +679,9 @@ export class FnExpression extends Expression {
       if (fn) {
         break;
       }
-      if (isPage(controller)) {
-        controller = undefined;
-        break;
-      }
-      const component = controller as Component;
-      controller = component.parent;
+      // controller could be a Component or a PageBase in practice,
+      // using `as Component` to avoid a runtime instanceof check.
+      controller = (controller as Component).parent;
     }
     const setFn = fn && fn.set;
     if (!setFn) throw new Error('No setter function for: ' + this.segments.join('.'));
diff --git a/src/templates/templates.ts b/src/templates/templates.ts
index c755ca23..1aec0195 100644
--- a/src/templates/templates.ts
+++ b/src/templates/templates.ts
@@ -2006,7 +2006,7 @@ export class DynamicViewInstance extends BaseViewInstance {
     const contextView = context.getView();
     const namespace = contextView && contextView.namespace;
     const view = name && context.meta.views.find(name, namespace);
-    return view || exports.emptyTemplate;
+    return view || emptyTemplate;
   }
 
   dependencies(context: Context, options: { ignoreTemplate?: Template; }) {
@@ -2203,7 +2203,7 @@ export class Views {
         // For views with the `server` option, serialize them with a blank
         // template body. This allows them to be used from other views on the
         // browser, but they will output nothing on the browser
-        if (view.options.server) template = exports.emptyTemplate;
+        if (view.options.server) template = emptyTemplate;
       }
       // Serializing views as a function allows them to be constructed lazily upon
       // first use. This can improve initial load times of the application when