From dece42dce3db0dceff48ad1e7810953554622472 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 18 Jul 2022 14:55:13 +0530 Subject: [PATCH] Do not store all the manifests in memory --- src/platform/web/ThemeBuilder.ts | 13 +++++-------- src/platform/web/ThemeLoader.ts | 18 +++++++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/platform/web/ThemeBuilder.ts b/src/platform/web/ThemeBuilder.ts index 85d1e336..2ff4108a 100644 --- a/src/platform/web/ThemeBuilder.ts +++ b/src/platform/web/ThemeBuilder.ts @@ -15,29 +15,26 @@ limitations under the License. */ import type {ThemeInformation} from "./ThemeLoader"; import type {Platform} from "./Platform.js"; +import type {ThemeManifest} from "../types/theme"; import {ColorSchemePreference} from "./ThemeLoader"; import {IconColorizer} from "./IconColorizer"; import {DerivedVariables} from "./DerivedVariables"; -import {ThemeManifest} from "../types/theme"; import {ILogItem} from "../../logging/types"; export class ThemeBuilder { - private _idToManifest: Map; private _themeMapping: Record = {}; private _preferredColorScheme?: ColorSchemePreference; private _platform: Platform; private _injectedVariables?: Record; - constructor(platform: Platform, manifestMap: Map, preferredColorScheme?: ColorSchemePreference) { - this._idToManifest = manifestMap; + constructor(platform: Platform, preferredColorScheme?: ColorSchemePreference) { this._preferredColorScheme = preferredColorScheme; this._platform = platform; } - async populateDerivedTheme(manifest: ThemeManifest, log: ILogItem): Promise { + async populateDerivedTheme(manifest: ThemeManifest, baseManifest: ThemeManifest, baseManifestLocation: string, log: ILogItem): Promise { await log.wrap("ThemeBuilder.populateThemeMap", async () => { - const {manifest: baseManifest, location} = this._idToManifest.get(manifest.extends!)!; - const {cssLocation, derivedVariables, icons} = this._getSourceData(baseManifest, location, log); + const {cssLocation, derivedVariables, icons} = this._getSourceData(baseManifest, baseManifestLocation, log); const themeName = manifest.name; if (!themeName) { throw new Error(`Theme name not found in manifest!`); @@ -49,7 +46,7 @@ export class ThemeBuilder { const { name: variantName, default: isDefault, dark, variables } = variantDetails; const resolvedVariables = new DerivedVariables(variables, derivedVariables, dark).toVariables(); Object.assign(variables, resolvedVariables); - const iconVariables = await new IconColorizer(this._platform, icons, variables, location).toVariables(); + const iconVariables = await new IconColorizer(this._platform, icons, variables, baseManifestLocation).toVariables(); Object.assign(variables, resolvedVariables, iconVariables); const themeDisplayName = `${themeName} ${variantName}`; if (isDefault) { diff --git a/src/platform/web/ThemeLoader.ts b/src/platform/web/ThemeLoader.ts index b899ab5e..81114a54 100644 --- a/src/platform/web/ThemeLoader.ts +++ b/src/platform/web/ThemeLoader.ts @@ -15,7 +15,7 @@ limitations under the License. */ import type {ILogItem} from "../../logging/types"; -import { ThemeManifest } from "../types/theme"; +import type {ThemeManifest} from "../types/theme"; import type {Platform} from "./Platform.js"; import {ThemeBuilder} from "./ThemeBuilder"; @@ -52,20 +52,24 @@ export class ThemeLoader { } async init(manifestLocations: string[], log?: ILogItem): Promise { - const idToManifest = new Map(); await this._platform.logger.wrapOrRun(log, "ThemeLoader.init", async (log) => { this._themeMapping = {}; const results = await Promise.all( - manifestLocations.map( location => this._platform.request(location, { method: "GET", format: "json", cache: true, }).response()) + manifestLocations.map(location => this._platform.request(location, { method: "GET", format: "json", cache: true, }).response()) ); - results.forEach(({ body }, i) => idToManifest.set(body.id, { manifest: body, location: manifestLocations[i] })); - this._themeBuilder = new ThemeBuilder(this._platform, idToManifest, this.preferredColorScheme); + this._themeBuilder = new ThemeBuilder(this._platform, this.preferredColorScheme); const runtimeThemePromises: Promise[] = []; for (let i = 0; i < results.length; ++i) { const { body } = results[i]; try { if (body.extends) { - const promise = this._themeBuilder.populateDerivedTheme(body, log); + const indexOfBaseManifest = results.findIndex(manifest => manifest.body.id === body.extends); + if (indexOfBaseManifest === -1) { + throw new Error(`Base manifest for derived theme at ${manifestLocations[i]} not found!`); + } + const {body: baseManifest} = results[indexOfBaseManifest]; + const baseManifestLocation = manifestLocations[indexOfBaseManifest]; + const promise = this._themeBuilder.populateDerivedTheme(body, baseManifest, baseManifestLocation, log); runtimeThemePromises.push(promise); } else { @@ -97,7 +101,7 @@ export class ThemeLoader { } private _populateThemeMap(manifest: ThemeManifest, manifestLocation: string, log: ILogItem) { - log.wrap("populateThemeMap", () => { + log.wrap("ThemeLoader.populateThemeMap", () => { /* After build has finished, the source section of each theme manifest contains `built-assets` which is a mapping from the theme-id to