Merge pull request #16199 from resilient-tech/perf-listfactory

perf: remove duplicate call to `ListView.show`

frappe/public/js/frappe/list/list_factory.js

@@ -6,8 +6,8 @@ frappe.provide('frappe.views.list_view');
 window.cur_list = null;
 frappe.views.ListFactory = class ListFactory extends frappe.views.Factory {
 	make (route) {
-		var me = this;
-		var doctype = route[1];
+		const me = this;
+		const doctype = route[1];
 
 		// List / Gantt / Kanban / etc
 		// File is a special view
@@ -21,60 +21,58 @@ frappe.views.ListFactory = class ListFactory extends frappe.views.Factory {
 		}
 
 		frappe.provide('frappe.views.list_view.' + doctype);
-		const page_name = frappe.get_route_str();
-
-		if (!frappe.views.list_view[page_name]) {
-			frappe.views.list_view[page_name] = new view_class({
-				doctype: doctype,
-				parent: me.make_page(true, page_name)
-			});
-		} else {
-			frappe.container.change_to(page_name);
-		}
-		me.set_cur_list();
 
+		frappe.views.list_view[me.page_name] = new view_class({
+			doctype: doctype,
+			parent: me.make_page(true, me.page_name)
+		});
 
+		me.set_cur_list();
 	}
 
-	show() {
+	before_show() {
 		if (this.re_route_to_view()) {
-			return;
+			return false;
 		}
+
 		this.set_module_breadcrumb();
-		super.show();
+	}
+
+	on_show() {
 		this.set_cur_list();
-		cur_list && cur_list.show();
+		if (cur_list) cur_list.show();
 	}
 
 	re_route_to_view() {
-		var route = frappe.get_route();
-		var doctype = route[1];
-		var last_route = frappe.route_history.slice(-2)[0];
-		if (route[0] === 'List' && route.length === 2 && frappe.views.list_view[doctype]) {
-			if(last_route && last_route[0]==='List' && last_route[1]===doctype) {
-				// last route same as this route, so going back.
-				// this happens because /app/List/Item will redirect to /app/List/Item/List
-				// while coming from back button, the last 2 routes will be same, so
-				// we know user is coming in the reverse direction (via back button)
+		const doctype = this.route[1];
+		const last_route = frappe.route_history.slice(-2)[0];
+		if (
+			this.route[0] === 'List' &&
+			this.route.length === 2	&&
+			frappe.views.list_view[doctype] &&
+			last_route &&
+			last_route[0] === 'List' &&
+			last_route[1] === doctype
+		) {
+			// last route same as this route, so going back.
+			// this happens because /app/List/Item will redirect to /app/List/Item/List
+			// while coming from back button, the last 2 routes will be same, so
+			// we know user is coming in the reverse direction (via back button)
 
-				// example:
-				// Step 1: /app/List/Item redirects to /app/List/Item/List
-				// Step 2: User hits "back" comes back to /app/List/Item
-				// Step 3: Now we cannot send the user back to /app/List/Item/List so go back one more step
-				window.history.go(-1);
-				return true;
-			} else {
-				return false;
-			}
+			// example:
+			// Step 1: /app/List/Item redirects to /app/List/Item/List
+			// Step 2: User hits "back" comes back to /app/List/Item
+			// Step 3: Now we cannot send the user back to /app/List/Item/List so go back one more step
+			window.history.go(-1);
+			return true;
 		}
 	}
 
 	set_module_breadcrumb() {
 		if (frappe.route_history.length > 1) {
-			var prev_route = frappe.route_history[frappe.route_history.length - 2];
+			const prev_route = frappe.route_history[frappe.route_history.length - 2];
 			if (prev_route[0] === 'modules') {
-				var doctype = frappe.get_route()[1],
-					module = prev_route[1];
+				const doctype = this.route[1], module = prev_route[1];
 				if (frappe.module_links[module] && frappe.module_links[module].includes(doctype)) {
 					// save the last page from the breadcrumb was accessed
 					frappe.breadcrumbs.set_doctype_module(doctype, module);
@@ -84,10 +82,8 @@ frappe.views.ListFactory = class ListFactory extends frappe.views.Factory {
 	}
 
 	set_cur_list() {
-		var route = frappe.get_route();
-		var page_name = frappe.get_route_str();
-		cur_list = frappe.views.list_view[page_name];
-		if (cur_list && cur_list.doctype !== route[1]) {
+		cur_list = frappe.views.list_view[this.page_name];
+		if (cur_list && cur_list.doctype !== this.route[1]) {
 			// changing...
 			window.cur_list = null;
 		}

frappe/public/js/frappe/views/factory.js

@@ -10,20 +10,21 @@ frappe.views.Factory = class Factory {
 	}
 
 	show() {
-		var page_name = frappe.get_route_str(),
-			me = this;
+		this.route = frappe.get_route();
+		this.page_name = frappe.get_route_str();
 
-		if (frappe.pages[page_name]) {
-			frappe.container.change_to(page_name);
-			if(me.on_show) {
-				me.on_show();
+		if (this.before_show && this.before_show() === false) return;
+
+		if (frappe.pages[this.page_name]) {
+			frappe.container.change_to(this.page_name);
+			if (this.on_show) {
+				this.on_show();
 			}
 		} else {
-			var route = frappe.get_route();
-			if(route[1]) {
-				me.make(route);
+			if (this.route[1]) {
+				this.make(this.route);
 			} else {
-				frappe.show_not_found(route);
+				frappe.show_not_found(this.route);
 			}
 		}
 	}
@@ -34,15 +35,17 @@ frappe.views.Factory = class Factory {
 }
 
 frappe.make_page = function(double_column, page_name) {
-	if(!page_name) {
-		var page_name = frappe.get_route_str();
+	if (!page_name) {
+		page_name = frappe.get_route_str();
 	}
-	var page = frappe.container.add_page(page_name);
+
+	const page = frappe.container.add_page(page_name);
 
 	frappe.ui.make_app_page({
 		parent: page,
 		single_column: !double_column
 	});
+
 	frappe.container.change_to(page_name);
 	return page;
 }