From c59b5b1efdc3a588fb8a13029a6593feab142e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20O=C3=9Fwald?= Date: Wed, 2 Jan 2019 13:19:48 +0100 Subject: [PATCH] [FIX] ComponentAnalyzer: Fully handle sap.ui5/routing (#124) - Detect routerClass dependency - Detect dependencies of all targets regardless of defined routes --- lib/lbt/analyzer/ComponentAnalyzer.js | 72 +++++----- test/lib/lbt/analyzer/ComponentAnalyzer.js | 146 ++++++++++++++++++--- 2 files changed, 158 insertions(+), 60 deletions(-) diff --git a/lib/lbt/analyzer/ComponentAnalyzer.js b/lib/lbt/analyzer/ComponentAnalyzer.js index 73bb1d248..2a2a760bc 100644 --- a/lib/lbt/analyzer/ComponentAnalyzer.js +++ b/lib/lbt/analyzer/ComponentAnalyzer.js @@ -33,13 +33,6 @@ function each(obj, fn) { } } -function makeArray(value) { - if ( Array.isArray(value) ) { - return value; - } - return value == null ? [] : [value]; -} - /** * Analyzes the manifest for a Component.js to find more dependencies. * @since 1.47.0 @@ -133,45 +126,44 @@ class ComponentAnalyzer { const routing = ui5.routing; if ( routing ) { - if (Array.isArray(routing.routes)) { - routing.routes.forEach((route) => this._visitRoute(route, routing, info)); - } else { - for (const key in routing.routes) { - if (!routing.routes.hasOwnProperty(key)) { - continue; - } - const route = routing.routes[key]; - this._visitRoute(route, routing, info); + const routingConfig = routing.config || {}; + + // See sap/ui/core/UIComponent#init + if (routing.routes) { + const routerClassName = routingConfig.routerClass || "sap.ui.core.routing.Router"; + const routerClassModule = ModuleName.fromUI5LegacyName(routerClassName); + log.verbose(`adding router dependency '${routerClassModule}'`); + info.addDependency(routerClassModule); + } else if (routing.targets) { + const targetsModule = routingConfig.targetsClass || "sap/ui/core/routing/Targets.js"; + log.verbose(`adding routing targets dependency '${targetsModule}'`); + info.addDependency(targetsModule); + + const viewsModule = "sap/ui/core/routing/Views.js"; + log.verbose(`adding routing views dependency '${viewsModule}'`); + info.addDependency(viewsModule); + } + + for (const targetName in routing.targets) { + if (!routing.targets.hasOwnProperty(targetName)) { + continue; + } + const target = routing.targets[targetName]; + if (target && target.viewName) { + // merge target config with default config + const config = Object.assign({}, routing.config, target); + const module = ModuleName.fromUI5LegacyName( + (config.viewPath ? config.viewPath + "." : "") + + config.viewName, ".view." + config.viewType.toLowerCase() ); + log.verbose("converting routing target '%s' to view dependency '%s'", targetName, module); + // TODO make this a conditional dependency, depending on the route pattern? + info.addDependency(module); } } } return info; } - - /** - * called for any route, this adds the view used by a route as a dependency. - * - * @param {object} route the single route - * @param {object} routing the full routing object from the manifest - * @param {ModuleInfo} info ModuleInfo object that should be enriched - * @private - */ - _visitRoute( route, routing, info ) { - makeArray(route.target).forEach((targetRef) => { - const target = routing.targets[targetRef]; - if ( target && target.viewName ) { - // merge target config with default config - const config = Object.assign({}, routing.config, target); - const module = ModuleName.fromUI5LegacyName( - (config.viewPath ? config.viewPath + "." : "") + - config.viewName, ".view." + config.viewType.toLowerCase() ); - log.verbose("converting routing target '%s' to view dependency '%s'", targetRef, module); - // TODO make this a conditional dependency, depending on the pattern? - info.addDependency(module); - } - }); - } } module.exports = ComponentAnalyzer; diff --git a/test/lib/lbt/analyzer/ComponentAnalyzer.js b/test/lib/lbt/analyzer/ComponentAnalyzer.js index 660e82f8d..525745997 100644 --- a/test/lib/lbt/analyzer/ComponentAnalyzer.js +++ b/test/lib/lbt/analyzer/ComponentAnalyzer.js @@ -19,7 +19,99 @@ function createMockPool(relPath, manifest) { }; } -test("routing with routes as array", (t) => { +test("routing with empty config, routes, targets", async (t) => { + const mockManifest = { + "sap.ui5": { + routing: { + config: {}, + routes: [], + targets: {} + } + } + }; + + const mockPool = createMockPool("test/", mockManifest); + + const mockInfo = { + deps: [], + addDependency(name) { + this.deps.push(name); + } + }; + + const subject = new ComponentAnalyzer(mockPool); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Router.js" + ], "dependencies should be correct"); +}); + +test("routing with empty config, targets", async (t) => { + const mockManifest = { + "sap.ui5": { + routing: { + config: {}, + targets: {} + } + } + }; + + const mockPool = createMockPool("test/", mockManifest); + + const mockInfo = { + deps: [], + addDependency(name) { + this.deps.push(name); + } + }; + + const subject = new ComponentAnalyzer(mockPool); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Targets.js", + "sap/ui/core/routing/Views.js" + ], "dependencies should be correct"); +}); + +test("routing with targets but no routes", async (t) => { + const mockManifest = { + "sap.ui5": { + routing: { + config: { + viewPath: "test.view", + viewType: "XML" + }, + targets: { + test: { + viewName: "App" + } + } + } + } + }; + + const mockPool = createMockPool("test/", mockManifest); + + const mockInfo = { + deps: [], + addDependency(name) { + this.deps.push(name); + } + }; + + const subject = new ComponentAnalyzer(mockPool); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Targets.js", + "sap/ui/core/routing/Views.js", + "test/view/App.view.xml" + ], "dependencies should be correct"); +}); + +test("routing with routes as array", async (t) => { const mockManifest = { "sap.ui5": { routing: { @@ -43,17 +135,23 @@ test("routing with routes as array", (t) => { const mockPool = createMockPool("test/", mockManifest); const mockInfo = { + deps: [], addDependency(name) { - t.is(name, "test/view/App.view.xml"); + this.deps.push(name); } }; const subject = new ComponentAnalyzer(mockPool); - return subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Router.js", + "test/view/App.view.xml" + ], "dependencies should be correct"); }); -test("routing with routes as object", (t) => { +test("routing with routes as object", async (t) => { const mockManifest = { "sap.ui5": { routing: { @@ -76,16 +174,22 @@ test("routing with routes as object", (t) => { const mockPool = createMockPool("test/", mockManifest); const mockInfo = { + deps: [], addDependency(name) { - t.is(name, "test/view/App.view.xml"); + this.deps.push(name); } }; const subject = new ComponentAnalyzer(mockPool); - return subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Router.js", + "test/view/App.view.xml" + ], "dependencies should be correct"); }); -test("routing with route with multiple targets", (t) => { +test("routing with route with multiple targets", async (t) => { const mockManifest = { "sap.ui5": { routing: { @@ -116,15 +220,16 @@ test("routing with route with multiple targets", (t) => { }; const subject = new ComponentAnalyzer(mockPool); - return subject.analyze({name: path.join("test", "Component.js")}, mockInfo).then( () => { - t.deepEqual(mockInfo.deps, [ - "test/view/Master.view.xml", - "test/view/Detail.view.xml" - ], "dependencies should be correct"); - }); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Router.js", + "test/view/Master.view.xml", + "test/view/Detail.view.xml" + ], "dependencies should be correct"); }); -test("routing with targets with local config", (t) => { +test("routing with targets with local config", async (t) => { const mockManifest = { "sap.ui5": { routing: { @@ -164,12 +269,13 @@ test("routing with targets with local config", (t) => { }; const subject = new ComponentAnalyzer(mockPool); - return subject.analyze({name: path.join("test", "Component.js")}, mockInfo).then( () => { - t.deepEqual(mockInfo.deps, [ - "test/view/Master.view.js", - "test/subview/Detail.view.xml" - ], "dependencies should be correct"); - }); + await subject.analyze({name: path.join("test", "Component.js")}, mockInfo); + + t.deepEqual(mockInfo.deps, [ + "sap/ui/core/routing/Router.js", + "test/view/Master.view.js", + "test/subview/Detail.view.xml" + ], "dependencies should be correct"); }); test("rootView with object", (t) => {