Skip to content

Defer evaluation of defaultCacheDirectory until it is needed#41683

Closed
Jobians wants to merge 2 commits into
microsoft:mainfrom
Jobians:patch-1
Closed

Defer evaluation of defaultCacheDirectory until it is needed#41683
Jobians wants to merge 2 commits into
microsoft:mainfrom
Jobians:patch-1

Conversation

@Jobians

@Jobians Jobians commented Jul 7, 2026

Copy link
Copy Markdown

Summary

This PR defers evaluation of "defaultCacheDirectory" by converting it from an eagerly evaluated value to a function and updating its call sites.

Motivation

"defaultCacheDirectory" was previously evaluated during module initialization. This meant that its platform checks were performed before callers had a chance to provide an explicit override, even when the default cache directory was never needed.

By evaluating the default cache directory only when it is actually required, the existing behavior is preserved while avoiding unnecessary initialization.

Changes

  • Convert "defaultCacheDirectory" to a lazily evaluated function.
  • Update callers to invoke "defaultCacheDirectory()" when constructing default paths.
  • No functional changes are intended for supported platforms; only the evaluation timing has changed.

Jobians added 2 commits July 7, 2026 21:41
Convert `defaultCacheDirectory` from an eagerly evaluated constant to a function and invoke it only when the default browser registry path is needed.

This avoids evaluating the default cache directory during module initialization and ensures the cache path is computed only when `PLAYWRIGHT_BROWSERS_PATH` is not set.

No behavior changes are intended for supported platforms; this only defers evaluation until it is required.

Signed-off-by: JOBIANS TECHIE <88005779+Jobians@users.noreply.github.com>
Update the MCP profile directory code to call `defaultCacheDirectory()` after converting it from an eagerly evaluated value to a function.

This keeps the call site consistent with the lazy evaluation introduced in the registry code.

Signed-off-by: JOBIANS TECHIE <88005779+Jobians@users.noreply.github.com>
@Jobians

Jobians commented Jul 7, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@Jobians

Jobians commented Jul 7, 2026

Copy link
Copy Markdown
Author

@yury-s

@pavelfeldman

Copy link
Copy Markdown
Member

Start with filing an issue, showing that there are hot execution paths that trigger this call without using the value.

@Jobians

Jobians commented Jul 8, 2026

Copy link
Copy Markdown
Author

The motivation for this change comes from a real regression I encountered.

Previously, this worked correctly in Termux. When I set "PLAYWRIGHT_BROWSERS_PATH=0", Playwright did not attempt to evaluate the default cache directory, so it never threw the "Unsupported platform: android" error.

After updating the package yesterday, I started getting that error. Looking into the changes, I noticed that the default cache directory is now being evaluated before "PLAYWRIGHT_BROWSERS_PATH" is respected. As a result, even when an explicit browser path is provided through the environment variable, the code still executes the Android platform check and fails.

This PR restores the previous behavior by deferring evaluation of the default cache directory until it is actually needed. When "PLAYWRIGHT_BROWSERS_PATH" is set, there is no reason to compute the default path.

Please take another look before closing this PR, as it addresses a real regression affecting Termux users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants