Skip to content

[browser] Remove custom cache #114901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

maraf
Copy link
Member

@maraf maraf commented Apr 22, 2025

The only remaining use-case for custom cache is when using standalone mode & having misconfigured HTTP server. In that case the behaviour of the custom cache can be recreated in user code.

Fixes #85775

@maraf maraf added arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm labels Apr 22, 2025
@maraf maraf added this to the 10.0.0 milestone Apr 22, 2025
@maraf maraf self-assigned this Apr 22, 2025
@maraf maraf marked this pull request as ready for review April 23, 2025 10:18
@Copilot Copilot AI review requested due to automatic review settings April 23, 2025 10:18
@maraf maraf requested a review from a team April 23, 2025 10:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the custom caching support from the browser runtime, as it is no longer needed for the remaining use-case. Key changes include conditional removal of cache settings in the boot JSON generation, elimination of caching helper functions from the TypeScript interfaces and globals, and complete removal of the assetsCache implementation.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/GenerateWasmBootJson.cs Added a conditional branch to only set cacheBootResources for legacy targets
src/mono/browser/runtime/types/internal.ts Removed LoaderHelpers functions related to custom cache logging and purging
src/mono/browser/runtime/types/index.ts Removed the cacheBootResources property from the MonoConfig type
src/mono/browser/runtime/startup.ts Removed calls to cache-related functions previously used during startup
src/mono/browser/runtime/loader/run.ts Removed cache initialization calls from the runtime loader
src/mono/browser/runtime/loader/globals.ts Removed cache helper imports from the loader globals
src/mono/browser/runtime/loader/assetsCache.ts Completely removed the custom cache implementation file
src/mono/browser/runtime/loader/assets.ts Updated resource download logic to eliminate usage of cache-based fetching
Files not reviewed (1)
  • src/mono/nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets: Language not supported
Comments suppressed due to low confidence (8)

src/mono/browser/runtime/types/internal.ts:166

  • The removal of 'logDownloadStatsToConsole' from LoaderHelpers may impact parts of the runtime expecting this functionality. Please verify that all consumers of LoaderHelpers have been updated accordingly.
-    logDownloadStatsToConsole: () => void;

src/mono/browser/runtime/types/internal.ts:167

  • Removing 'purgeUnusedCacheEntriesAsync' could affect resource cache maintenance; ensure that any dependent code now operates correctly without it.
-    purgeUnusedCacheEntriesAsync: () => Promise<void>;

src/mono/browser/runtime/types/index.ts:120

  • The removal of 'cacheBootResources' from MonoConfig should be reviewed to ensure that all configuration-dependent code has been updated accordingly.
-    cacheBootResources?: boolean,

src/mono/browser/runtime/startup.ts:338

  • The deletion of the block invoking cache-related logging and purge functions must be validated so that startup behavior remains correct without custom cache support.
-        if (loaderHelpers.config.debugLevel !== 0 && loaderHelpers.config.cacheBootResources) {

src/mono/browser/runtime/loader/run.ts:511

  • Removing the cache initialization call aligns with the removal of custom caching; please confirm that this does not adversely affect asset loading in any expected scenarios.
-    await initCacheToUseIfEnabled();

src/mono/browser/runtime/loader/globals.ts:122

  • Ensure that the removal of cache helper functions from the globals does not leave any stale references in the codebase.
-        logDownloadStatsToConsole,

src/mono/browser/runtime/loader/assetsCache.ts:1

  • Since the entire assetsCache.ts file has been removed, please verify that all modules previously importing this file have been updated to no longer reference any custom cache logic.
-// Licensed to the .NET Foundation under one or more agreements.

src/mono/browser/runtime/loader/assets.ts:676

  • Replacing the call to 'download_resource_with_cache' with 'fetchResource' appropriately removes custom caching; ensure that this change is thoroughly tested in all asset loading scenarios.
-        const fetchResponse = download_resource_with_cache(asset);

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

How are we planning to detect that webassembly is available in auto mode? (it essentially check for wasm availability and starts server if not or starts wasm directly if available)

@maraf maraf added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-Build-mono NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[browser] Remove .NET specific cache
4 participants