diff --git a/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cs b/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cs index 827679b9ac8..4aa73aba06f 100644 --- a/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cs +++ b/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cs @@ -12,13 +12,6 @@ namespace Bit.BlazorUI; public partial class BitMap : BitComponentBase where TMapProvider : class, IBitMapProvider, new() { - /// - /// Clears the process-wide script/stylesheet load cache. Intended for unit tests only - - /// production code should not need to invalidate the cache because the browser already - /// dedupes the underlying network requests. - /// - public static void ResetAssetLoadCacheForTesting() => BitMapAssetCache.Reset(); - private bool _initialized; private string _canvasId = string.Empty; private string _helpId = string.Empty; @@ -2030,22 +2023,24 @@ private async ValueTask SwapProviderAsync(TMapProvider effective) } /// - /// Loads the provider's stylesheets and scripts, deduped process-wide. + /// Loads the provider's stylesheets and scripts. /// Returns false when the scripts could not be loaded, which means the map cannot be built. /// private async ValueTask LoadAssetsAsync(TMapProvider provider) { - // Process-wide dedup so mounting/unmounting multiple BitMaps over the same provider - // doesn't pay an interop round-trip per mount. The browser dedupes by URL too, but - // skipping the round-trip avoids serialising the URL list and waiting for a JS - // promise that does nothing useful. - var pendingStylesheets = BitMapAssetCache.FilterUnloadedStylesheets(provider.Stylesheets); - if (pendingStylesheets.Count > 0) + // Every mount asks, and the dedup lives on the JS side on purpose: it caches per + // document - by the in-flight promise and by scanning what the document already + // holds - which is the only scope that is right in every render mode. A .NET-side + // cache would have to be per document too, and on Blazor Server a process serves + // many of them: the first document to mount a map would mark the URLs loaded and + // every later one would be told there is nothing to inject and fail on a provider + // global that was never defined. What the round-trip costs is serialising the URL + // list and awaiting a promise that resolves at once. + if (provider.Stylesheets.Count > 0) { try { - await _js.BitExtrasInitStylesheets(pendingStylesheets); - BitMapAssetCache.MarkStylesheetsLoaded(pendingStylesheets); + await _js.BitExtrasInitStylesheets(provider.Stylesheets); } catch (Exception ex) { @@ -2058,13 +2053,11 @@ private async ValueTask LoadAssetsAsync(TMapProvider provider) if (Gone) return false; - var pendingScripts = BitMapAssetCache.FilterUnloadedScripts(provider.Scripts); - if (pendingScripts.Count > 0) + if (provider.Scripts.Count > 0) { try { - await _js.BitExtrasInitScripts(pendingScripts, provider.ScriptsAreModules); - BitMapAssetCache.MarkScriptsLoaded(pendingScripts); + await _js.BitExtrasInitScripts(provider.Scripts, provider.ScriptsAreModules); } catch (Exception ex) { diff --git a/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapAssetCache.cs b/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapAssetCache.cs deleted file mode 100644 index c46ce4f9ab8..00000000000 --- a/src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapAssetCache.cs +++ /dev/null @@ -1,57 +0,0 @@ -using System.Collections.Concurrent; - -namespace Bit.BlazorUI; - -/// -/// Process-wide record of which provider script / stylesheet URLs have already been -/// requested by any . -/// -/// This lives on a non-generic type on purpose. Static state declared inside -/// BitMap<TMapProvider> is per closed generic, so two maps over different -/// provider types that happen to share a CDN URL would each pay their own interop -/// round-trip. The browser dedupes the underlying network request either way; what the -/// cache saves is serialising the URL list and awaiting a JS promise that does nothing. -/// -/// -internal static class BitMapAssetCache -{ - // Keyed case-sensitively - URLs are case-sensitive on most servers. - private static readonly ConcurrentDictionary _scripts = new(StringComparer.Ordinal); - private static readonly ConcurrentDictionary _stylesheets = new(StringComparer.Ordinal); - - /// Returns the subset of that has not been requested yet. - public static List FilterUnloadedScripts(IReadOnlyList urls) => Filter(urls, _scripts); - - /// Returns the subset of that has not been requested yet. - public static List FilterUnloadedStylesheets(IReadOnlyList urls) => Filter(urls, _stylesheets); - - /// Marks scripts as loaded. Called only after the JS side reported success. - public static void MarkScriptsLoaded(IEnumerable urls) - { - foreach (var url in urls) _scripts[url] = 1; - } - - /// Marks stylesheets as loaded. Called only after the JS side reported success. - public static void MarkStylesheetsLoaded(IEnumerable urls) - { - foreach (var url in urls) _stylesheets[url] = 1; - } - - /// Clears the cache. Intended for unit tests only. - public static void Reset() - { - _scripts.Clear(); - _stylesheets.Clear(); - } - - private static List Filter(IReadOnlyList urls, ConcurrentDictionary cache) - { - if (urls.Count == 0) return []; - var result = new List(urls.Count); - foreach (var url in urls) - { - if (cache.ContainsKey(url) is false) result.Add(url); - } - return result; - } -} diff --git a/src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts b/src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts index 8fbbcb050b8..4e21394c9db 100644 --- a/src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts +++ b/src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts @@ -276,78 +276,83 @@ namespace BitBlazorUI { } } + // Both loaders cache per asset, not per list: two components asking for + // overlapping-but-unequal lists have to await the very same load, instead of one of + // them finding the other's still-loading tag in the document and taking it for ready. + private static _initScriptsPromises: { [key: string]: Promise } = {}; public static async initScripts(scripts: string[], isModule: boolean) { - const key = scripts.join('|'); + return Promise.all(scripts.map(s => Extras.initScript(s, isModule))); + } + + private static initScript(url: string, isModule: boolean) { + const key = `${isModule ? 'module' : 'script'}|${url}`; if (Extras._initScriptsPromises[key] !== undefined) { return Extras._initScriptsPromises[key]; } const allScripts = Array.from(document.scripts).map(s => s.src); - const notAddedScripts = scripts.filter(s => !allScripts.find(as => as.includes(s))); + if (allScripts.find(as => as.includes(url))) return Promise.resolve(); - if (notAddedScripts.length == 0) return Promise.resolve(); - - const promise = new Promise(async (res: any, rej: any) => { - try { - await Promise.all(notAddedScripts.map(addScript)); - res(); - } catch (e: any) { - rej(e); + const promise = new Promise((res, rej) => { + const script = document.createElement('script'); + script.src = url; + if (isModule) { + script.type = 'module'; } + script.onload = res; + script.onerror = e => { + // The dead tag goes out with the forgotten promise, otherwise the scan + // above would find it and take a retry for an already loaded script. + script.remove(); + rej(e); + }; + document.body.appendChild(script); }); - Extras._initScriptsPromises[key] = promise; - return promise; - - async function addScript(url: string) { - return new Promise((res, rej) => { - const script = document.createElement('script'); - script.src = url; - if (isModule) { - script.type = 'module'; - } - script.onload = res; - script.onerror = rej; - document.body.appendChild(script); - }) - } + // A rejected load is not remembered: caching it would make one CDN hiccup + // permanent for the life of the document, so a later mount could never retry. + Extras._initScriptsPromises[key] = promise.catch((e: any) => { + delete Extras._initScriptsPromises[key]; + throw e; + }); + return Extras._initScriptsPromises[key]; } private static _initStylesheetsPromises: { [key: string]: Promise } = {}; - public static async initStylesheets(stylesheets: string[], isModule: boolean) { - const key = stylesheets.join('|'); - if (Extras._initStylesheetsPromises[key] !== undefined) { - return Extras._initStylesheetsPromises[key]; - } - - const allStylesheets = Array.from(document.links).filter(l => l.rel === 'stylesheet').map(s => s.href); - const notAddedStylesheets = stylesheets.filter(s => !allStylesheets.find(as => as.includes(s))); + public static async initStylesheets(stylesheets: string[]) { + return Promise.all(stylesheets.map(s => Extras.initStylesheet(s))); + } - if (notAddedStylesheets.length == 0) return Promise.resolve(); + private static initStylesheet(url: string) { + if (Extras._initStylesheetsPromises[url] !== undefined) { + return Extras._initStylesheetsPromises[url]; + } - const promise = new Promise(async (res: any, rej: any) => { - try { - await Promise.all(notAddedStylesheets.map(addStylesheet)); - res(); - } catch (e: any) { + // Queried, not read off document.links, which holds only and elements. + const links = document.querySelectorAll('link[rel="stylesheet"]'); + const allStylesheets = Array.from(links).map(l => l.href); + if (allStylesheets.find(as => as.includes(url))) return Promise.resolve(); + + const promise = new Promise((res, rej) => { + const link = document.createElement('link'); + link.href = url; + link.rel = 'stylesheet'; + link.onload = res; + link.onerror = e => { + link.remove(); rej(e); - } + }; + document.head.appendChild(link); }); - Extras._initStylesheetsPromises[key] = promise; - return promise; - - async function addStylesheet(url: string) { - return new Promise((res, rej) => { - const link = document.createElement('link'); - link.href = url; - link.rel = 'stylesheet'; - link.onload = res; - link.onerror = rej; - document.head.appendChild(link); - }) - } + // A rejected load is not remembered: caching it would make one CDN hiccup + // permanent for the life of the document, so a later mount could never retry. + Extras._initStylesheetsPromises[url] = promise.catch((e: any) => { + delete Extras._initStylesheetsPromises[url]; + throw e; + }); + return Extras._initStylesheetsPromises[url]; } diff --git a/src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs b/src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs index b5d74b9b3e0..a49b1a9a403 100644 --- a/src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs +++ b/src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs @@ -50,17 +50,6 @@ public class BitMapTests : BunitTestContext private const string GET_VIEW = "BitBlazorUI.BitMapLeaflet.getView"; private const string PROJECT = "BitBlazorUI.BitMapLeaflet.project"; - [TestInitialize] - public void ResetAssetCache() - { - // BitMap dedupes script / stylesheet loads process-wide so the same provider URL - // isn't re-requested when multiple maps mount in quick succession. Tests that - // assert on initScripts/initStylesheets invocations need to reset that cache so - // each test starts from a clean state. - BitMap.ResetAssetLoadCacheForTesting(); - BitMap.ResetAssetLoadCacheForTesting(); - } - /// /// Sets up the two capability probes a mount makes, so the map reaches its Ready state. /// @@ -116,13 +105,16 @@ public void BitMapShouldLoadStylesheetsAndScriptsBeforeInit() } [TestMethod] - public void BitMapShouldDedupeAssetLoadsAcrossMounts() + public void BitMapShouldRequestAssetsOnEveryMount() { + // Dedup belongs to the JS side, which caches per document. A .NET-side cache would + // be process-wide, and on Blazor Server one process serves many documents: the + // second document would be told its provider scripts were already injected and fail + // on a global that was never defined there. So every mount asks. Context.JSInterop.SetupVoid(INIT_STYLESHEETS); Context.JSInterop.SetupVoid(INIT_SCRIPTS); Context.JSInterop.SetupVoid(INIT); - // First mount: stylesheets + scripts must be requested. RenderComponent>(); var firstStylesheetCalls = Context.JSInterop.Invocations.Count(i => i.Identifier == INIT_STYLESHEETS); var firstScriptCalls = Context.JSInterop.Invocations.Count(i => i.Identifier == INIT_SCRIPTS); @@ -130,15 +122,14 @@ public void BitMapShouldDedupeAssetLoadsAcrossMounts() Assert.IsTrue(firstStylesheetCalls >= 1, "First mount should request stylesheets"); Assert.IsTrue(firstScriptCalls >= 1, "First mount should request scripts"); - // Second mount: cache should kick in and skip redundant load round-trips. RenderComponent>(); - Assert.AreEqual(firstStylesheetCalls, + Assert.AreEqual(firstStylesheetCalls * 2, Context.JSInterop.Invocations.Count(i => i.Identifier == INIT_STYLESHEETS), - "Second mount must not re-request already-loaded stylesheets"); - Assert.AreEqual(firstScriptCalls, + "Second mount must request stylesheets again"); + Assert.AreEqual(firstScriptCalls * 2, Context.JSInterop.Invocations.Count(i => i.Identifier == INIT_SCRIPTS), - "Second mount must not re-request already-loaded scripts"); + "Second mount must request scripts again"); } [TestMethod] @@ -645,11 +636,12 @@ public void BitMapShouldNotWaitForVisibilityWhenLazyLoadIsDisabled() } [TestMethod] - public void BitMapShouldShareTheAssetCacheAcrossProviderTypes() + public void BitMapShouldAskForTheSameUrlFromEveryProviderType() { - // The cache lives on a non-generic type on purpose: static state inside - // BitMap is per closed generic, so two maps over different provider types - // that share a script URL would each pay their own interop round-trip. + // Two provider types sharing a script URL both ask for it, each with the URL itself as + // the request. That is what lets the JS side collapse them: it keys its cache on the URL + // list and scans what the document already holds, so nothing about the dedup depends on + // which closed generic asked, or on a .NET static outliving the document. Context.JSInterop.SetupVoid(INIT_STYLESHEETS); Context.JSInterop.SetupVoid(INIT_SCRIPTS); Context.JSInterop.SetupVoid("BitBlazorUI.SharedAssetProviderA.init"); @@ -659,13 +651,18 @@ public void BitMapShouldShareTheAssetCacheAcrossProviderTypes() Context.JSInterop.Setup(CHROME_REDUCED_MOTION).SetResult(false); RenderComponent>(); - var afterFirst = Context.JSInterop.Invocations.Count(i => i.Identifier == INIT_SCRIPTS); - RenderComponent>(); - Assert.AreEqual(1, afterFirst); - Assert.AreEqual(afterFirst, Context.JSInterop.Invocations.Count(i => i.Identifier == INIT_SCRIPTS), - "A second provider type sharing the same script URL must not re-request it"); + var scriptCalls = Context.JSInterop.Invocations.Where(i => i.Identifier == INIT_SCRIPTS).ToList(); + + Assert.AreEqual(2, scriptCalls.Count, "Each provider type must request its scripts"); + CollectionAssert.AreEqual( + new[] { "https://cdn.example.com/shared-map.js" }, + ((IEnumerable)scriptCalls[0].Arguments[0]!).ToArray()); + CollectionAssert.AreEqual( + ((IEnumerable)scriptCalls[0].Arguments[0]!).ToArray(), + ((IEnumerable)scriptCalls[1].Arguments[0]!).ToArray(), + "Both must ask under the same URL, which is the key the JS side dedupes on"); } [TestMethod]