From c838c7251d1beea542a0977707202e62683c344e Mon Sep 17 00:00:00 2001 From: Anders Mikkelsen Date: Mon, 19 Jun 2023 17:17:10 +0200 Subject: [PATCH 1/6] Added first draft at registerLazy --- src/core/application.ts | 6 ++++++ src/core/router.ts | 25 +++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/core/application.ts b/src/core/application.ts index b8b92715..f544ea43 100644 --- a/src/core/application.ts +++ b/src/core/application.ts @@ -7,6 +7,8 @@ import { Router } from "./router" import { Schema, defaultSchema } from "./schema" import { ActionDescriptorFilter, ActionDescriptorFilters, defaultActionDescriptorFilters } from "./action_descriptor" +export type AsyncConstructor = () => Promise + export class Application implements ErrorHandler { readonly element: Element readonly schema: Schema @@ -49,6 +51,10 @@ export class Application implements ErrorHandler { this.load({ identifier, controllerConstructor }) } + registerLazy(identifier: string, controllerConstructor: AsyncConstructor) { + this.router.addLazy(identifier, controllerConstructor) + } + registerActionOption(name: string, filter: ActionDescriptorFilter) { this.actionDescriptorFilters[name] = filter } diff --git a/src/core/router.ts b/src/core/router.ts index 6b63318d..c3d05d06 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -1,4 +1,4 @@ -import { Application } from "./application" +import { Application, AsyncConstructor } from "./application" import { Context } from "./context" import { Definition } from "./definition" import { Module } from "./module" @@ -11,11 +11,13 @@ export class Router implements ScopeObserverDelegate { private scopeObserver: ScopeObserver private scopesByIdentifier: Multimap private modulesByIdentifier: Map + private lazyModulesByIdentifier: Map constructor(application: Application) { this.application = application this.scopeObserver = new ScopeObserver(this.element, this.schema, this) this.scopesByIdentifier = new Multimap() + this.lazyModulesByIdentifier = new Map() this.modulesByIdentifier = new Map() } @@ -87,11 +89,30 @@ export class Router implements ScopeObserverDelegate { return new Scope(this.schema, element, identifier, this.logger) } + addLazy(identifier: string, controllerConstructor: AsyncConstructor) { + this.lazyModulesByIdentifier.set(identifier, controllerConstructor) + } + scopeConnected(scope: Scope) { - this.scopesByIdentifier.add(scope.identifier, scope) + const { identifier } = scope + this.scopesByIdentifier.add(identifier, scope) const module = this.modulesByIdentifier.get(scope.identifier) if (module) { module.connectContextForScope(scope) + } else if (this.lazyModulesByIdentifier.has(identifier)) { + const callback = this.lazyModulesByIdentifier.get(identifier) + if (callback) { + callback().then((controllerConstructor) => { + this.loadDefinition({ identifier, controllerConstructor }) + this.lazyModulesByIdentifier.delete(identifier) + }) + } else { + this.application.logger.warn( + `Simulus expected the callback registered for "${identifier}" to resolve to a controllerConstructor but didn't`, + `Failed to lazy load ${identifier}`, + { identifier } + ) + } } } From 06158cdcb707074a58ae2b16d1e6fb44f182eecc Mon Sep 17 00:00:00 2001 From: Anders Mikkelsen Date: Mon, 19 Jun 2023 17:55:23 +0200 Subject: [PATCH 2/6] Fixed issue concurrently connecting controllers --- src/core/router.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/router.ts b/src/core/router.ts index e0fb38d7..4c3df5c2 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -113,8 +113,10 @@ export class Router implements ScopeObserverDelegate { const callback = this.lazyModulesByIdentifier.get(identifier) if (callback) { callback().then((controllerConstructor) => { - this.loadDefinition({ identifier, controllerConstructor }) - this.lazyModulesByIdentifier.delete(identifier) + if (!this.modulesByIdentifier.has(identifier)) { + this.loadDefinition({ identifier, controllerConstructor }) + this.lazyModulesByIdentifier.delete(identifier) + } }) } else { this.application.logger.warn( From cfbc51226969c78f3686cb093df6907d945270ac Mon Sep 17 00:00:00 2001 From: Anders Mikkelsen Date: Mon, 19 Jun 2023 18:27:48 +0200 Subject: [PATCH 3/6] Added spec --- src/core/router.ts | 2 +- src/tests/modules/core/error_handler_tests.ts | 2 +- src/tests/modules/core/lazy_loading_tests.ts | 28 +++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 src/tests/modules/core/lazy_loading_tests.ts diff --git a/src/core/router.ts b/src/core/router.ts index 4c3df5c2..5e67981b 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -111,7 +111,7 @@ export class Router implements ScopeObserverDelegate { module.connectContextForScope(scope) } else if (this.lazyModulesByIdentifier.has(identifier)) { const callback = this.lazyModulesByIdentifier.get(identifier) - if (callback) { + if (callback && typeof callback === "function") { callback().then((controllerConstructor) => { if (!this.modulesByIdentifier.has(identifier)) { this.loadDefinition({ identifier, controllerConstructor }) diff --git a/src/tests/modules/core/error_handler_tests.ts b/src/tests/modules/core/error_handler_tests.ts index d94482ea..00a956c1 100644 --- a/src/tests/modules/core/error_handler_tests.ts +++ b/src/tests/modules/core/error_handler_tests.ts @@ -2,7 +2,7 @@ import { Controller } from "../../../core/controller" import { Application } from "../../../core/application" import { ControllerTestCase } from "../../cases/controller_test_case" -class MockLogger { +export class MockLogger { errors: any[] = [] logs: any[] = [] warns: any[] = [] diff --git a/src/tests/modules/core/lazy_loading_tests.ts b/src/tests/modules/core/lazy_loading_tests.ts new file mode 100644 index 00000000..b82548bc --- /dev/null +++ b/src/tests/modules/core/lazy_loading_tests.ts @@ -0,0 +1,28 @@ +import { ApplicationTestCase } from "../../cases" +import { Controller } from "../../../core" +import { MockLogger } from "./error_handler_tests" + +class LazyController extends Controller { + connect() { + this.application.logger.log("Hello from lazy controller") + } +} + +export default class LazyLoadingTests extends ApplicationTestCase { + async setupApplication() { + this.application.logger = new MockLogger() + + this.application.registerLazy("lazy", () => new Promise((resolve, _reject) => resolve(LazyController))) + } + + get mockLogger(): MockLogger { + return this.application.logger as any + } + + async "test lazy loading of controllers"() { + await this.renderFixture(`
`) + + this.assert.equal(this.mockLogger.logs.length, 2) + this.mockLogger.logs.forEach((entry) => this.assert.equal(entry, "Hello from lazy controller")) + } +} From 3ae44baf2b1aab2a26694fbe1df8b16d1a2b2a2f Mon Sep 17 00:00:00 2001 From: Anders Mikkelsen Date: Mon, 19 Jun 2023 19:15:07 +0200 Subject: [PATCH 4/6] refactored to method --- src/core/router.ts | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/core/router.ts b/src/core/router.ts index 5e67981b..4b3cbb0f 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -110,21 +110,7 @@ export class Router implements ScopeObserverDelegate { if (module) { module.connectContextForScope(scope) } else if (this.lazyModulesByIdentifier.has(identifier)) { - const callback = this.lazyModulesByIdentifier.get(identifier) - if (callback && typeof callback === "function") { - callback().then((controllerConstructor) => { - if (!this.modulesByIdentifier.has(identifier)) { - this.loadDefinition({ identifier, controllerConstructor }) - this.lazyModulesByIdentifier.delete(identifier) - } - }) - } else { - this.application.logger.warn( - `Simulus expected the callback registered for "${identifier}" to resolve to a controllerConstructor but didn't`, - `Failed to lazy load ${identifier}`, - { identifier } - ) - } + this.loadLazyModule(identifier) } } @@ -149,4 +135,22 @@ export class Router implements ScopeObserverDelegate { const scopes = this.scopesByIdentifier.getValuesForKey(module.identifier) scopes.forEach((scope) => module.disconnectContextForScope(scope)) } + + private loadLazyModule(identifier: string) { + const callback = this.lazyModulesByIdentifier.get(identifier) + if (callback && typeof callback === "function") { + callback().then((controllerConstructor) => { + if (!this.modulesByIdentifier.has(identifier)) { + this.loadDefinition({ identifier, controllerConstructor }) + this.lazyModulesByIdentifier.delete(identifier) + } + }) + } else { + this.application.logger.warn( + `Simulus expected the callback registered for "${identifier}" to resolve to a controllerConstructor but didn't`, + `Failed to lazy load ${identifier}`, + { identifier } + ) + } + } } From 8458157e5cdcdbd1d507c820382b8c718cfb7bcb Mon Sep 17 00:00:00 2001 From: Anders Mikkelsen Date: Mon, 19 Jun 2023 19:19:22 +0200 Subject: [PATCH 5/6] Warning when already registered a controller --- src/core/router.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/router.ts b/src/core/router.ts index 4b3cbb0f..ac7aee1f 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -100,7 +100,11 @@ export class Router implements ScopeObserverDelegate { } addLazy(identifier: string, controllerConstructor: AsyncConstructor) { - this.lazyModulesByIdentifier.set(identifier, controllerConstructor) + if (!this.modulesByIdentifier.has(identifier) && !this.lazyModulesByIdentifier.has(identifier)) { + this.lazyModulesByIdentifier.set(identifier, controllerConstructor) + } else { + this.application.logger.warn(`Stimulus has already a controller with "${identifier}" registered.`) + } } scopeConnected(scope: Scope) { @@ -147,7 +151,7 @@ export class Router implements ScopeObserverDelegate { }) } else { this.application.logger.warn( - `Simulus expected the callback registered for "${identifier}" to resolve to a controllerConstructor but didn't`, + `Stimulus expected the callback registered for "${identifier}" to resolve to a controllerConstructor but didn't`, `Failed to lazy load ${identifier}`, { identifier } ) From 98a279a057ba62c8d4adb11a7058d64180921074 Mon Sep 17 00:00:00 2001 From: Anders Mikkelsen Date: Mon, 19 Jun 2023 19:21:28 +0200 Subject: [PATCH 6/6] better naming and moved method underneath modules comment --- src/core/application.ts | 2 +- src/core/router.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/application.ts b/src/core/application.ts index f544ea43..d707ff43 100644 --- a/src/core/application.ts +++ b/src/core/application.ts @@ -52,7 +52,7 @@ export class Application implements ErrorHandler { } registerLazy(identifier: string, controllerConstructor: AsyncConstructor) { - this.router.addLazy(identifier, controllerConstructor) + this.router.registerLazyModule(identifier, controllerConstructor) } registerActionOption(name: string, filter: ActionDescriptorFilter) { diff --git a/src/core/router.ts b/src/core/router.ts index ac7aee1f..5f3099bf 100644 --- a/src/core/router.ts +++ b/src/core/router.ts @@ -99,14 +99,6 @@ export class Router implements ScopeObserverDelegate { return new Scope(this.schema, element, identifier, this.logger) } - addLazy(identifier: string, controllerConstructor: AsyncConstructor) { - if (!this.modulesByIdentifier.has(identifier) && !this.lazyModulesByIdentifier.has(identifier)) { - this.lazyModulesByIdentifier.set(identifier, controllerConstructor) - } else { - this.application.logger.warn(`Stimulus has already a controller with "${identifier}" registered.`) - } - } - scopeConnected(scope: Scope) { const { identifier } = scope this.scopesByIdentifier.add(identifier, scope) @@ -128,6 +120,14 @@ export class Router implements ScopeObserverDelegate { // Modules + registerLazyModule(identifier: string, controllerConstructor: AsyncConstructor) { + if (!this.modulesByIdentifier.has(identifier) && !this.lazyModulesByIdentifier.has(identifier)) { + this.lazyModulesByIdentifier.set(identifier, controllerConstructor) + } else { + this.application.logger.warn(`Stimulus has already a controller with "${identifier}" registered.`) + } + } + private connectModule(module: Module) { this.modulesByIdentifier.set(module.identifier, module) const scopes = this.scopesByIdentifier.getValuesForKey(module.identifier)