From e3372f0f2bfe991156cd1dce4872610ba2076977 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 20 Jun 2022 12:54:18 +0530 Subject: [PATCH 1/6] Don't use theme-name in manifest file names --- scripts/build-plugins/rollup-plugin-build-themes.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/build-plugins/rollup-plugin-build-themes.js b/scripts/build-plugins/rollup-plugin-build-themes.js index 43a21623..e74d3b85 100644 --- a/scripts/build-plugins/rollup-plugin-build-themes.js +++ b/scripts/build-plugins/rollup-plugin-build-themes.js @@ -257,8 +257,10 @@ module.exports = function buildThemes(options) { const derivedVariables = compiledVariables["derived-variables"]; const icon = compiledVariables["icon"]; const builtAssets = {}; + let themeKey; for (const chunk of chunkArray) { const [, name, variant] = chunk.fileName.match(/theme-(.+)-(.+)\.css/); + themeKey = name; builtAssets[`${name}-${variant}`] = assetMap.get(chunk.fileName).fileName; } manifest.source = { @@ -267,7 +269,7 @@ module.exports = function buildThemes(options) { "derived-variables": derivedVariables, "icon": icon }; - const name = `theme-${manifest.name}.json`; + const name = `theme-${themeKey}.json`; manifestLocations.push(`assets/${name}`); this.emitFile({ type: "asset", From 93165cb947ee0ad895e24d15c0ce5410afde3668 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 20 Jun 2022 13:46:14 +0530 Subject: [PATCH 2/6] runtime theme chunks should also be stored in map There will be more than one runtime theme file when multiple theme collections exist. --- .../build-plugins/rollup-plugin-build-themes.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/scripts/build-plugins/rollup-plugin-build-themes.js b/scripts/build-plugins/rollup-plugin-build-themes.js index e74d3b85..41e78044 100644 --- a/scripts/build-plugins/rollup-plugin-build-themes.js +++ b/scripts/build-plugins/rollup-plugin-build-themes.js @@ -46,7 +46,7 @@ function addThemesToConfig(bundle, manifestLocations, defaultThemes) { function parseBundle(bundle) { const chunkMap = new Map(); const assetMap = new Map(); - let runtimeThemeChunk; + let runtimeThemeChunkMap = new Map(); for (const [fileName, info] of Object.entries(bundle)) { if (!fileName.endsWith(".css")) { continue; @@ -60,18 +60,18 @@ function parseBundle(bundle) { assetMap.set(info.name, info); continue; } + const location = info.facadeModuleId?.match(/(.+)\/.+\.css/)?.[1]; + if (!location) { + throw new Error("Cannot find location of css chunk!"); + } if (info.facadeModuleId?.includes("type=runtime")) { /** * We have a separate field in manifest.source just for the runtime theme, * so store this separately. */ - runtimeThemeChunk = info; + runtimeThemeChunkMap.set(location, info); continue; } - const location = info.facadeModuleId?.match(/(.+)\/.+\.css/)?.[1]; - if (!location) { - throw new Error("Cannot find location of css chunk!"); - } const array = chunkMap.get(location); if (!array) { chunkMap.set(location, [info]); @@ -80,7 +80,7 @@ function parseBundle(bundle) { array.push(info); } } - return { chunkMap, assetMap, runtimeThemeChunk }; + return { chunkMap, assetMap, runtimeThemeChunkMap }; } module.exports = function buildThemes(options) { @@ -249,7 +249,7 @@ module.exports = function buildThemes(options) { // assetMap: Mapping from asset-name (eg: element-dark.css) to AssetInfo // chunkMap: Mapping from theme-location (eg: hydrogen-web/src/.../css/themes/element) to a list of ChunkInfo // types of AssetInfo and ChunkInfo can be found at https://rollupjs.org/guide/en/#generatebundle - const { assetMap, chunkMap, runtimeThemeChunk } = parseBundle(bundle); + const { assetMap, chunkMap, runtimeThemeChunkMap } = parseBundle(bundle); const manifestLocations = []; for (const [location, chunkArray] of chunkMap) { const manifest = require(`${location}/manifest.json`); @@ -263,6 +263,7 @@ module.exports = function buildThemes(options) { themeKey = name; builtAssets[`${name}-${variant}`] = assetMap.get(chunk.fileName).fileName; } + const runtimeThemeChunk = runtimeThemeChunkMap.get(location); manifest.source = { "built-assets": builtAssets, "runtime-asset": assetMap.get(runtimeThemeChunk.fileName).fileName, From 5eec72471217f0770101598659bf4f1e603d7005 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 20 Jun 2022 20:35:06 +0530 Subject: [PATCH 3/6] Locations must be relative to manifest --- scripts/build-plugins/rollup-plugin-build-themes.js | 11 ++++++++--- src/platform/web/Platform.js | 2 +- src/platform/web/ThemeLoader.ts | 11 ++++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/build-plugins/rollup-plugin-build-themes.js b/scripts/build-plugins/rollup-plugin-build-themes.js index 41e78044..89eaeec1 100644 --- a/scripts/build-plugins/rollup-plugin-build-themes.js +++ b/scripts/build-plugins/rollup-plugin-build-themes.js @@ -251,6 +251,8 @@ module.exports = function buildThemes(options) { // types of AssetInfo and ChunkInfo can be found at https://rollupjs.org/guide/en/#generatebundle const { assetMap, chunkMap, runtimeThemeChunkMap } = parseBundle(bundle); const manifestLocations = []; + // Location of the directory containing manifest relative to the root of the build output + const manifestLocation = "assets"; for (const [location, chunkArray] of chunkMap) { const manifest = require(`${location}/manifest.json`); const compiledVariables = options.compiledVariables.get(location); @@ -261,17 +263,20 @@ module.exports = function buildThemes(options) { for (const chunk of chunkArray) { const [, name, variant] = chunk.fileName.match(/theme-(.+)-(.+)\.css/); themeKey = name; - builtAssets[`${name}-${variant}`] = assetMap.get(chunk.fileName).fileName; + const locationRelativeToBuildRoot = assetMap.get(chunk.fileName).fileName; + const locationRelativeToManifest = path.relative(manifestLocation, locationRelativeToBuildRoot); + builtAssets[`${name}-${variant}`] = locationRelativeToManifest; } const runtimeThemeChunk = runtimeThemeChunkMap.get(location); + const runtimeAssetLocation = path.relative(manifestLocation, assetMap.get(runtimeThemeChunk.fileName).fileName); manifest.source = { "built-assets": builtAssets, - "runtime-asset": assetMap.get(runtimeThemeChunk.fileName).fileName, + "runtime-asset": runtimeAssetLocation, "derived-variables": derivedVariables, "icon": icon }; const name = `theme-${themeKey}.json`; - manifestLocations.push(`assets/${name}`); + manifestLocations.push(`${manifestLocation}/${name}`); this.emitFile({ type: "asset", name, diff --git a/src/platform/web/Platform.js b/src/platform/web/Platform.js index e2ca2028..c2eef17e 100644 --- a/src/platform/web/Platform.js +++ b/src/platform/web/Platform.js @@ -338,7 +338,7 @@ export class Platform { document.querySelectorAll(".theme").forEach(e => e.remove()); // add new theme const styleTag = document.createElement("link"); - styleTag.href = `./${newPath}`; + styleTag.href = newPath; styleTag.rel = "stylesheet"; styleTag.type = "text/css"; styleTag.className = "theme"; diff --git a/src/platform/web/ThemeLoader.ts b/src/platform/web/ThemeLoader.ts index 8c9364bc..ee303b49 100644 --- a/src/platform/web/ThemeLoader.ts +++ b/src/platform/web/ThemeLoader.ts @@ -61,11 +61,11 @@ export class ThemeLoader { const results = await Promise.all( manifestLocations.map( location => this._platform.request(location, { method: "GET", format: "json", cache: true, }).response()) ); - results.forEach(({ body }) => this._populateThemeMap(body, log)); + results.forEach(({ body }, i) => this._populateThemeMap(body, manifestLocations[i], log)); }); } - private _populateThemeMap(manifest, log: ILogItem) { + private _populateThemeMap(manifest, manifestLocation: string, log: ILogItem) { log.wrap("populateThemeMap", (l) => { /* After build has finished, the source section of each theme manifest @@ -75,7 +75,12 @@ export class ThemeLoader { const builtAssets: Record = manifest.source?.["built-assets"]; const themeName = manifest.name; let defaultDarkVariant: any = {}, defaultLightVariant: any = {}; - for (const [themeId, cssLocation] of Object.entries(builtAssets)) { + for (let [themeId, cssLocation] of Object.entries(builtAssets)) { + /** + * This cssLocation is relative to the location of the manifest file. + * So we first need to resolve it relative to the root of this hydrogen instance. + */ + cssLocation = new URL(cssLocation, new URL(manifestLocation, window.location.origin)).href; const variant = themeId.match(/.+-(.+)/)?.[1]; const { name: variantName, default: isDefault, dark } = manifest.values.variants[variant!]; const themeDisplayName = `${themeName} ${variantName}`; From fbdd512e0646bdc036f94b858d1d6172f06c13a4 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 20 Jun 2022 21:10:11 +0530 Subject: [PATCH 4/6] Split functions into smaller functions --- .../rollup-plugin-build-themes.js | 64 ++++++++++++++----- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/scripts/build-plugins/rollup-plugin-build-themes.js b/scripts/build-plugins/rollup-plugin-build-themes.js index 89eaeec1..9ac2befe 100644 --- a/scripts/build-plugins/rollup-plugin-build-themes.js +++ b/scripts/build-plugins/rollup-plugin-build-themes.js @@ -43,10 +43,39 @@ function addThemesToConfig(bundle, manifestLocations, defaultThemes) { } } -function parseBundle(bundle) { +/** + * Returns a mapping from location (of manifest file) to an array containing all the chunks (of css files) generated from that location. + * To understand what chunk means in this context, see https://rollupjs.org/guide/en/#generatebundle. + * @param {*} bundle Mapping from fileName to AssetInfo | ChunkInfo + */ +function getMappingFromLocationToChunkArray(bundle) { const chunkMap = new Map(); + for (const [fileName, info] of Object.entries(bundle)) { + if (!fileName.endsWith(".css") || info.type === "asset" || info.facadeModuleId?.includes("type=runtime")) { + continue; + } + const location = info.facadeModuleId?.match(/(.+)\/.+\.css/)?.[1]; + if (!location) { + throw new Error("Cannot find location of css chunk!"); + } + const array = chunkMap.get(location); + if (!array) { + chunkMap.set(location, [info]); + } + else { + array.push(info); + } + } + return chunkMap; +} + +/** + * Returns a mapping from unhashed file name (of css files) to AssetInfo. + * To understand what AssetInfo means in this context, see https://rollupjs.org/guide/en/#generatebundle. + * @param {*} bundle Mapping from fileName to AssetInfo | ChunkInfo + */ +function getMappingFromFileNameToAssetInfo(bundle) { const assetMap = new Map(); - let runtimeThemeChunkMap = new Map(); for (const [fileName, info] of Object.entries(bundle)) { if (!fileName.endsWith(".css")) { continue; @@ -58,6 +87,20 @@ function parseBundle(bundle) { * searching through the bundle array later. */ assetMap.set(info.name, info); + } + } + return assetMap; +} + +/** + * Returns a mapping from location (of manifest file) to ChunkInfo of the runtime css asset + * To understand what ChunkInfo means in this context, see https://rollupjs.org/guide/en/#generatebundle. + * @param {*} bundle Mapping from fileName to AssetInfo | ChunkInfo + */ +function getMappingFromLocationToRuntimeChunk(bundle) { + let runtimeThemeChunkMap = new Map(); + for (const [fileName, info] of Object.entries(bundle)) { + if (!fileName.endsWith(".css") || info.type === "asset") { continue; } const location = info.facadeModuleId?.match(/(.+)\/.+\.css/)?.[1]; @@ -70,17 +113,9 @@ function parseBundle(bundle) { * so store this separately. */ runtimeThemeChunkMap.set(location, info); - continue; - } - const array = chunkMap.get(location); - if (!array) { - chunkMap.set(location, [info]); - } - else { - array.push(info); } } - return { chunkMap, assetMap, runtimeThemeChunkMap }; + return runtimeThemeChunkMap; } module.exports = function buildThemes(options) { @@ -246,10 +281,9 @@ module.exports = function buildThemes(options) { }, generateBundle(_, bundle) { - // assetMap: Mapping from asset-name (eg: element-dark.css) to AssetInfo - // chunkMap: Mapping from theme-location (eg: hydrogen-web/src/.../css/themes/element) to a list of ChunkInfo - // types of AssetInfo and ChunkInfo can be found at https://rollupjs.org/guide/en/#generatebundle - const { assetMap, chunkMap, runtimeThemeChunkMap } = parseBundle(bundle); + const assetMap = getMappingFromFileNameToAssetInfo(bundle); + const chunkMap = getMappingFromLocationToChunkArray(bundle); + const runtimeThemeChunkMap = getMappingFromLocationToRuntimeChunk(bundle); const manifestLocations = []; // Location of the directory containing manifest relative to the root of the build output const manifestLocation = "assets"; From d688fa47375e999326bb0e1c6a61ca7b203b6bd6 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Thu, 23 Jun 2022 15:06:22 +0530 Subject: [PATCH 5/6] Get the theme-collection id from manifest --- .../build-plugins/rollup-plugin-build-themes.js | 17 ++++++++++------- .../web/ui/css/themes/element/manifest.json | 1 + vite.config.js | 4 +--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/scripts/build-plugins/rollup-plugin-build-themes.js b/scripts/build-plugins/rollup-plugin-build-themes.js index 9ac2befe..6b8fdae0 100644 --- a/scripts/build-plugins/rollup-plugin-build-themes.js +++ b/scripts/build-plugins/rollup-plugin-build-themes.js @@ -123,6 +123,7 @@ module.exports = function buildThemes(options) { let isDevelopment = false; const virtualModuleId = '@theme/' const resolvedVirtualModuleId = '\0' + virtualModuleId; + const themeToManifestLocation = new Map(); return { name: "build-themes", @@ -137,20 +138,22 @@ module.exports = function buildThemes(options) { async buildStart() { if (isDevelopment) { return; } const { themeConfig } = options; - for (const [name, location] of Object.entries(themeConfig.themes)) { + for (const location of themeConfig.themes) { manifest = require(`${location}/manifest.json`); + const themeCollectionId = manifest.id; + themeToManifestLocation.set(themeCollectionId, location); variants = manifest.values.variants; for (const [variant, details] of Object.entries(variants)) { - const fileName = `theme-${name}-${variant}.css`; - if (name === themeConfig.default && details.default) { + const fileName = `theme-${themeCollectionId}-${variant}.css`; + if (themeCollectionId === themeConfig.default && details.default) { // This is the default theme, stash the file name for later if (details.dark) { defaultDark = fileName; - defaultThemes["dark"] = `${name}-${variant}`; + defaultThemes["dark"] = `${themeCollectionId}-${variant}`; } else { defaultLight = fileName; - defaultThemes["light"] = `${name}-${variant}`; + defaultThemes["light"] = `${themeCollectionId}-${variant}`; } } // emit the css as built theme bundle @@ -164,7 +167,7 @@ module.exports = function buildThemes(options) { this.emitFile({ type: "chunk", id: `${location}/theme.css?type=runtime`, - fileName: `theme-${name}-runtime.css`, + fileName: `theme-${themeCollectionId}-runtime.css`, }); } }, @@ -187,7 +190,7 @@ module.exports = function buildThemes(options) { if (theme === "default") { theme = options.themeConfig.default; } - const location = options.themeConfig.themes[theme]; + const location = themeToManifestLocation.get(theme); const manifest = require(`${location}/manifest.json`); const variants = manifest.values.variants; if (!variant || variant === "default") { diff --git a/src/platform/web/ui/css/themes/element/manifest.json b/src/platform/web/ui/css/themes/element/manifest.json index e183317c..cb21eaad 100644 --- a/src/platform/web/ui/css/themes/element/manifest.json +++ b/src/platform/web/ui/css/themes/element/manifest.json @@ -1,6 +1,7 @@ { "version": 1, "name": "Element", + "id": "element", "values": { "variants": { "light": { diff --git a/vite.config.js b/vite.config.js index 72be0184..10348218 100644 --- a/vite.config.js +++ b/vite.config.js @@ -33,9 +33,7 @@ export default defineConfig(({mode}) => { plugins: [ themeBuilder({ themeConfig: { - themes: { - element: "./src/platform/web/ui/css/themes/element", - }, + themes: ["./src/platform/web/ui/css/themes/element"], default: "element", }, compiledVariables, From 6718198d9cc9e5aac8d9e3c0ca5534440b53bdcd Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 11 Jul 2022 12:40:24 +0530 Subject: [PATCH 6/6] Continue with other items if this throws --- src/platform/web/ThemeLoader.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/platform/web/ThemeLoader.ts b/src/platform/web/ThemeLoader.ts index ee303b49..89430663 100644 --- a/src/platform/web/ThemeLoader.ts +++ b/src/platform/web/ThemeLoader.ts @@ -76,11 +76,16 @@ export class ThemeLoader { const themeName = manifest.name; let defaultDarkVariant: any = {}, defaultLightVariant: any = {}; for (let [themeId, cssLocation] of Object.entries(builtAssets)) { - /** - * This cssLocation is relative to the location of the manifest file. - * So we first need to resolve it relative to the root of this hydrogen instance. - */ - cssLocation = new URL(cssLocation, new URL(manifestLocation, window.location.origin)).href; + try { + /** + * This cssLocation is relative to the location of the manifest file. + * So we first need to resolve it relative to the root of this hydrogen instance. + */ + cssLocation = new URL(cssLocation, new URL(manifestLocation, window.location.origin)).href; + } + catch { + continue; + } const variant = themeId.match(/.+-(.+)/)?.[1]; const { name: variantName, default: isDefault, dark } = manifest.values.variants[variant!]; const themeDisplayName = `${themeName} ${variantName}`;