Skip to content

Fix getStores endpointURL under production minification - #419

Open
sirdeggen wants to merge 1 commit into
mainfrom
fix/storage-endpoint-url-minify
Open

Fix getStores endpointURL under production minification#419
sirdeggen wants to merge 1 commit into
mainfrom
fix/storage-endpoint-url-minify

Conversation

@sirdeggen

Copy link
Copy Markdown
Contributor

Summary

  • WalletStorageManager.getStoreEndpointURL used constructor.name === 'StorageClient', which fails after Vite/esbuild/webpack minify renames the class.
  • Remote backups still synced (walks _backups), but clients that match stores by URL (e.g. bsv-desktop Make Primary) saw endpointURL: undefined and failed with "No storage provider matching …".
  • Resolve the URL by duck-typing provider.endpointUrl instead.

Changes

  • WalletStorageManager.getStoreEndpointURL: duck-type endpointUrl; document why constructor.name is unsafe
  • Unit tests for normal, minified class name, local/empty, and getStores() exposure
  • CHANGELOG note under unreleased

Test plan

  • pnpm exec jest --runTestsByPath test/storage/getStoreEndpointURL.test.ts (5 passed)
  • After publish/link into bsv-desktop: add remote backup, Sync works, Make Primary on https://store-us-1.bsvb.tech succeeds

Production minifiers rename StorageClient, so getStores().endpointURL was
always undefined for remote providers while backup sync still worked.
Duck-type provider.endpointUrl instead.
@sonarqubecloud

Copy link
Copy Markdown

@ty-everett ty-everett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The implementation itself looks sound: duck-typing the existing endpointUrl property avoids the minified-class-name failure and covers both remote storage client variants. On this exact head I verified the focused suite (5/5), wallet-toolbox typecheck, package lint, and git diff --check locally. SonarCloud is also green with zero new findings and zero security hotspots.

There are still merge blockers that need an updated head:

  • This branch no longer merges cleanly with main; please rebase and resolve the wallet-toolbox CHANGELOG conflict while preserving both entries.
  • The required CI merge gate is red. Build, lint, and policy failed in docs:examples with unresolved workspace packages, the downstream test/coverage jobs were skipped, and Mutation quality plus build-and-test (24.x) consequently failed. Please rerun CI after the rebase and ensure the full required matrix—not only Sonar and the focused test—is green.

Once the branch is current and the complete CI/merge gate passes, I see no code-level blocker in this patch.

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