perf: defer heavy Dash SDK chunks to cut initial load time#28
Merged
Conversation
Dynamically import @dashevo/evo-sdk, dpns, contract, client, fee-estimator, and islock modules instead of eagerly bundling them with the main chunk, and load the CAP widget on demand rather than from a blocking script tag. An idle-time warmup primes these modules after first paint so user-initiated flows still feel instant, and the Vite modulePreload policy is adjusted to skip preload links for the heavy SDK chunks. DPNS label utilities are extracted to dpns-utils.ts with a Vitest suite, and the npm test script now runs once by default with test:watch reserved for the watcher. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a Vitest suite for the cached module loader and the Vite modulepreload filter to lock in the heavy-chunk exclusion contract, plus a check-build-artifacts.mjs script (wired up as the test:build-artifacts npm script) that asserts the built index.html and entry chunk never statically pull in the deferred Dash SDK chunks. Also covers getStepDescription so the user-facing key-generation copy can't silently drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire the lazy-Dash-chunk guard into CI between the Build and Test steps so a regression in modulepreload filtering or a static SDK import in the entry bundle fails the pipeline. Failure output now uses the GitHub Actions ::error annotation and a plain banner so the cause is visible from both the workflow summary and raw logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test script is already "vitest run", so passing --run --passWithNoTests through npm just duplicates the run flag and silently allows an empty suite. Letting npm test drive the invocation keeps CI honest if the suite ever disappears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR defers EvoSDK and other heavy module imports via cached dynamic loaders, schedules on-demand warmup on user intent, extracts DPNS utilities with tests, makes the CAP widget load-on-demand, and adds a Vite preload filter plus a CI smoke-check to prevent preloading heavy Dash chunks. ChangesLazy SDK Loading and Build Optimization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
scripts/check-build-artifacts.mjs (1)
52-53: ⚡ Quick winConsider refactoring the static import regex for maintainability.
The
staticImportPatternis complex and attempts to match both bare imports (import "...") and named imports (import X from "..."). While functionally correct, the duplicated chunk-name list and complex alternation make it fragile. Consider extracting the chunk names to a shared constant or building the regex programmatically.♻️ Proposed refactor to reduce duplication
+const heavyChunkNames = 'evo-sdk|dapi-client|dashcore-lib|dapi-subscription|islock'; +const heavyChunkPattern = new RegExp(`(?:${heavyChunkNames})`); -const heavyChunkPattern = /(?:evo-sdk|dapi-client|dashcore-lib|dapi-subscription|islock)/; // ...later... -const staticImportPattern = - /\bimport\s*(?:["'][^"']*(?:evo-sdk|dapi-client|dashcore-lib|dapi-subscription|islock)[^"']*["']|[\w*{}\s,]+from\s*["'][^"']*(?:evo-sdk|dapi-client|dashcore-lib|dapi-subscription|islock)[^"']*["'])/; +const staticImportPattern = new RegExp( + `\\bimport\\s*(?:["'][^"']*(?:${heavyChunkNames})[^"']*["']|[\\w*{}\\s,]+from\\s*["'][^"']*(?:${heavyChunkNames})[^"']*["'])` +);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-build-artifacts.mjs` around lines 52 - 53, The staticImportPattern regex is fragile due to duplicated chunk-name lists and complex alternation; extract the list of module chunk names (e.g., "evo-sdk", "dapi-client", "dashcore-lib", "dapi-subscription", "islock") into a single constant (e.g., CHUNK_NAMES or CHUNK_ALTERNATION) and programmatically build the regex for staticImportPattern so both bare imports (import "x") and named/import-from forms (import ... from "x") reuse that single source of truth; update the code that defines staticImportPattern to construct the final RegExp using the shared constant and verify it still matches both import variants.src/platform/sdkModule.ts (1)
5-12: ⚡ Quick winConsider using
createCachedLoaderto eliminate duplication.The manual promise-caching and rejection-handling logic here duplicates the pattern already extracted in
createCachedLoader(loaders.ts lines 1-12). Replacing this with:export const loadSdkModule = createCachedLoader(() => import('`@dashevo/evo-sdk`'));would reduce duplication and ensure both SDK and module loaders follow the same retry/cache semantics.
♻️ Proposed refactor
+import { createCachedLoader } from './loaders.js'; + export type SdkModule = typeof import('`@dashevo/evo-sdk`'); -let sdkModulePromise: Promise<SdkModule> | null = null; - -export function loadSdkModule(): Promise<SdkModule> { - if (!sdkModulePromise) { - sdkModulePromise = import('`@dashevo/evo-sdk`').catch((err) => { - sdkModulePromise = null; - throw err; - }); - } - return sdkModulePromise; -} +export const loadSdkModule = createCachedLoader(() => import('`@dashevo/evo-sdk`'));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/sdkModule.ts` around lines 5 - 12, The loadSdkModule implementation duplicates the manual promise-caching and rejection-handling logic already provided by createCachedLoader; replace the current loadSdkModule function with a cached loader by exporting loadSdkModule using createCachedLoader(() => import('`@dashevo/evo-sdk`')), so the module uses the shared retry/cache semantics and removes the custom sdkModulePromise handling in loadSdkModule; ensure you remove the old function and any related sdkModulePromise variable to avoid dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/faucet.ts`:
- Around line 122-150: In loadCapWidget, the retry hang happens because a failed
<script data-cap-widget> element remains in the DOM and subsequent querySelector
finds it but its load/error events already fired; update the error handling for
both the existing script branch (the existing variable used in querySelector)
and the newly created script (the script variable) so that on 'error' you remove
the failing element from the DOM (existing.remove() or script.remove()), reset
capWidgetPromise = null, and then reject with the error; ensure the .catch
cleanup remains (or consolidate cleanup into the error handlers) so retries will
re-inject a fresh tag and the promise can settle.
- Around line 136-142: Pin the CDN URL and add a load timeout for the script
element to avoid indefinite waiting: replace the unpinned script.src string used
when creating the script element (`script.src`) with a pinned version (e.g.
'`@cap.js/widget`@<version>') and wrap the load/error handlers with a timeout (use
setTimeout to reject after N ms). In the script load flow (the code that calls
`document.createElement('script')`, `script.addEventListener('load', ...)`,
`script.addEventListener('error', ...)`, and the Promise resolve/reject), ensure
you clear the timeout on both load and error, remove listeners, and call reject
with a descriptive Error on timeout so `solveCap` (or the function that awaits
this promise) will fail fast instead of hanging.
In `@src/config/vite-preload.test.ts`:
- Around line 6-11: Add one assertion to the existing test that verifies the new
"islock" heavy chunk is filtered: call shouldPreloadDashChunk with a
representative islock asset filename (e.g., 'assets/islock-<hash>.js') and
expect false; this ensures the updated HEAVY_DASH_CHUNK_PATTERN change is
covered by the test.
In `@src/config/vite-preload.ts`:
- Line 1: HEAVY_DASH_CHUNK_PATTERN currently omits the "islock" identifier
causing a mismatch with the smoke check's heavyChunkPattern; update the regular
expression referenced as HEAVY_DASH_CHUNK_PATTERN so it includes "islock"
alongside the existing alternatives (e.g.,
evo-sdk|dapi-client|dashcore-lib|dapi-subscription) so Vite's modulePreload
filter will also treat islock as a heavy chunk and keep behavior consistent with
scripts/check-build-artifacts.mjs.
---
Nitpick comments:
In `@scripts/check-build-artifacts.mjs`:
- Around line 52-53: The staticImportPattern regex is fragile due to duplicated
chunk-name lists and complex alternation; extract the list of module chunk names
(e.g., "evo-sdk", "dapi-client", "dashcore-lib", "dapi-subscription", "islock")
into a single constant (e.g., CHUNK_NAMES or CHUNK_ALTERNATION) and
programmatically build the regex for staticImportPattern so both bare imports
(import "x") and named/import-from forms (import ... from "x") reuse that single
source of truth; update the code that defines staticImportPattern to construct
the final RegExp using the shared constant and verify it still matches both
import variants.
In `@src/platform/sdkModule.ts`:
- Around line 5-12: The loadSdkModule implementation duplicates the manual
promise-caching and rejection-handling logic already provided by
createCachedLoader; replace the current loadSdkModule function with a cached
loader by exporting loadSdkModule using createCachedLoader(() =>
import('`@dashevo/evo-sdk`')), so the module uses the shared retry/cache semantics
and removes the custom sdkModulePromise handling in loadSdkModule; ensure you
remove the old function and any related sdkModulePromise variable to avoid dead
code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5268a0f-cc58-422c-b548-d61afb33a211
📒 Files selected for processing (21)
.github/workflows/ci.ymlindex.htmlpackage.jsonscripts/check-build-artifacts.mjssrc/api/faucet.tssrc/config/vite-preload.test.tssrc/config/vite-preload.tssrc/main.tssrc/platform/client.tssrc/platform/contract.tssrc/platform/dpns-utils.test.tssrc/platform/dpns-utils.tssrc/platform/dpns.tssrc/platform/identity.tssrc/platform/loaders.test.tssrc/platform/loaders.tssrc/platform/sdkModule.tssrc/ui/components.tssrc/ui/state.test.tssrc/ui/state.tsvite.config.ts
💤 Files with no reviewable changes (1)
- index.html
Pin the CAP widget CDN to a specific version, add a 30s load timeout, and remove the script tag on failure so a retry can inject a fresh one. Also exclude the islock chunk from preload alongside the other heavy Dash modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/api/faucet.test.ts (1)
76-109: 💤 Low valueOptional: cover the reuse-existing-script and "loaded without exposing Cap" branches.
The two tests exercise error→retry and timeout, but
loadCapWidgethas two untested branches that are easy to regress: reusing an already-presentscript[data-cap-widget](theexistingpath), and theloadevent firing whileCapis still undefined (rejects with "CAP widget loaded without exposing Cap"). Since the fake document already tracks scripts andquerySelector, adding these cases is low-cost and locks in the loader's idempotency/guard behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/faucet.test.ts` around lines 76 - 109, Add two unit tests for loadCapWidget/solveCap: one that simulates an existing <script data-cap-widget> in the fake document (reuse-existing-script path) so calling solveCap or loadCapWidget returns/uses the existing script rather than injecting another, and assert no duplicate script is created and behavior is correct; and another that simulates the <script> firing a 'load' event while the global Cap is still undefined to exercise the "CAP widget loaded without exposing Cap" branch (expect a rejection with that message and ensure the script is removed). Use the existing helpers installFakeDocument, installCap, and reference loadCapWidget/solveCap and the script[data-cap-widget] attribute to locate the code paths to test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/api/faucet.test.ts`:
- Around line 76-109: Add two unit tests for loadCapWidget/solveCap: one that
simulates an existing <script data-cap-widget> in the fake document
(reuse-existing-script path) so calling solveCap or loadCapWidget returns/uses
the existing script rather than injecting another, and assert no duplicate
script is created and behavior is correct; and another that simulates the
<script> firing a 'load' event while the global Cap is still undefined to
exercise the "CAP widget loaded without exposing Cap" branch (expect a rejection
with that message and ensure the script is removed). Use the existing helpers
installFakeDocument, installCap, and reference loadCapWidget/solveCap and the
script[data-cap-widget] attribute to locate the code paths to test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d780cba1-2f35-4c1c-9408-6c8427a7f25f
📒 Files selected for processing (4)
src/api/faucet.test.tssrc/api/faucet.tssrc/config/vite-preload.test.tssrc/config/vite-preload.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/config/vite-preload.test.ts
- src/config/vite-preload.ts
- src/api/faucet.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@dashevo/evo-sdk, DAPI client, dashcore-lib, fee-estimator, DPNS, contract, and islock modules instead of bundling them into the initial chunk. An idle-time warmup (viarequestIdleCallback/setTimeoutfallback) primes them after first paint so user-initiated flows stay snappy, and the CAP widget is loaded on demand rather than from a blocking<script>tag inindex.html.modulePreload.resolveDependenciesnow filters those heavy chunks out of the generated<link rel="modulepreload">tags via a smallshouldPreloadDashChunkhelper, so the browser doesn't pre-fetch them on first paint.src/platform/dpns-utils.tswith a dedicated Vitest suite, and split a reusablecreateCachedLoaderout ofloaders.ts(covered by tests for in-flight dedupe and post-failure retry).scripts/check-build-artifacts.mjs(wired up asnpm run test:build-artifactsand run in CI between Build and Test) that asserts the builtindex.htmlhas no heavymodulepreloadentries and that the entry chunk never statically imports a deferred module.npm testnow runs once by default (vitest run) withtest:watchfor the watcher, and CI usesnpm testdirectly instead of duplicating vitest flags.Impact
Lighthouse on the same flow:
(Before run is prod
bridge.thepasta.org; after run is localvite preview— environment isn't identical, but the byte-weight and main-thread numbers are environment-independent.)Test plan
npm run buildsucceedsnpm testpasses (loaders, vite-preload filter, DPNS utils, state)npm run test:build-artifactspasses; manually verified it fails if either (a) the VitemodulePreloadfilter is removed or (b) a heavy SDK module is statically imported from the entry?contract=deep-link flows still work; warmup kicks in after first paintSummary by CodeRabbit
New Features
Bug Fixes
Performance
Chores
Tests