fix(fathom-analytics): drop proxy support, bundle-only with sdkPatches#722
fix(fathom-analytics): drop proxy support, bundle-only with sdkPatches#722
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughProxying and Partytown forwarding for the Fathom Analytics integration are removed while bundling support is retained. Bundle capability now accepts Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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 docstrings
🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/script/src/plugins/transform.ts (1)
133-169:⚠️ Potential issue | 🔴 CriticalApply bundle-only
sdkPatcheseven when no proxy rewrites exist.Line 460 now passes Fathom’s bundle-only patch, but Line 164 only invokes
rewriteScriptUrlsASTwhenproxyRewrites?.lengthis truthy. For Fathom there is no proxy config, so the neutralize-domain-check patch is never applied. The cache key also ignoressdkPatches, so an old unpatchedbundle:${filename}can be reused.🐛 Proposed fix
- const proxyRewritesHash = proxyRewrites?.length ? `-${ohash(proxyRewrites)}` : '' - const cacheKey = proxyRewrites?.length ? `bundle-proxy:${filename.replace('.js', `${proxyRewritesHash}.js`)}` : `bundle:${filename}` + const hasScriptRewrites = !!(proxyRewrites?.length || sdkPatches?.length) + const scriptRewritesHash = hasScriptRewrites + ? `-${ohash({ proxyRewrites: proxyRewrites ?? [], sdkPatches: sdkPatches ?? [] })}` + : '' + const cacheKey = hasScriptRewrites + ? `bundle-proxy:${filename.replace('.js', `${scriptRewritesHash}.js`)}` + : `bundle:${filename}` @@ - // Apply URL rewrites for proxy mode (AST-based at build time) - if (proxyRewrites?.length && res) { + // Apply URL rewrites and SDK patches at build time. + // sdkPatches can be bundle-only, without proxy rewrites. + if ((proxyRewrites?.length || sdkPatches?.length) && res) { const content = res.toString('utf-8') - const rewritten = rewriteScriptUrlsAST(content, filename || 'script.js', proxyRewrites, sdkPatches, { skipApiRewrites, neutralizeCanvas }) + const rewritten = rewriteScriptUrlsAST(content, filename || 'script.js', proxyRewrites ?? [], sdkPatches, { skipApiRewrites, neutralizeCanvas }) res = Buffer.from(rewritten, 'utf-8') - logger.debug(`Rewrote ${proxyRewrites.length} URL patterns in ${filename}`) + logger.debug(`Applied script rewrites/patches in ${filename}`) }Also applies to: 456-476
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/plugins/transform.ts` around lines 133 - 169, The bundle-only SDK patches are never applied unless proxyRewrites exist and the cache key ignores sdkPatches, so ensure sdkPatches are considered both for rewriting and caching: include a hash of sdkPatches in proxyRewritesHash/cacheKey computation (so cacheKey reflects sdkPatches), always call rewriteScriptUrlsAST with sdkPatches (even when proxyRewrites is empty) before writing to storage, and use the same computed cacheKey when calling storage.setItemRaw instead of the hardcoded `bundle:${filename}`; update references to functions/vars proxyRewritesHash, cacheKey, shouldUseCache, storage.setItemRaw, rewriteScriptUrlsAST, and sdkPatches/neutralizeCanvas accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/script/src/registry.ts`:
- Around line 119-124: Remove Partytown proxying for Fathom: in the m(...)
registration for the fathom integration (the m call with id 'fathomAnalytics'
and feature 'useScriptFathomAnalytics') remove the partytown: true flag from the
options so Fathom beacons are not routed through Partytown, and also delete the
corresponding Partytown forwards entry that forwards requests to
cdn.usefathom.com (the Partytown forwards configuration that references
cdn.usefathom.com or the '/_scripts/p' proxy path) so Fathom beacons remain
direct and are not proxied.
---
Outside diff comments:
In `@packages/script/src/plugins/transform.ts`:
- Around line 133-169: The bundle-only SDK patches are never applied unless
proxyRewrites exist and the cache key ignores sdkPatches, so ensure sdkPatches
are considered both for rewriting and caching: include a hash of sdkPatches in
proxyRewritesHash/cacheKey computation (so cacheKey reflects sdkPatches), always
call rewriteScriptUrlsAST with sdkPatches (even when proxyRewrites is empty)
before writing to storage, and use the same computed cacheKey when calling
storage.setItemRaw instead of the hardcoded `bundle:${filename}`; update
references to functions/vars proxyRewritesHash, cacheKey, shouldUseCache,
storage.setItemRaw, rewriteScriptUrlsAST, and sdkPatches/neutralizeCanvas
accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7335518e-3ddd-4dad-87bd-3ab52e193bfc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
docs/content/scripts/fathom-analytics.mdexamples/cookie-consent/package.jsonexamples/custom-script/package.jsonexamples/granular-consent/package.jsonexamples/performance/package.jsonpackages/devtools-app/nuxt.config.tspackages/devtools-app/package.jsonpackages/script/src/plugins/rewrite-ast.tspackages/script/src/plugins/transform.tspackages/script/src/registry.tspackages/script/src/runtime/types.tspnpm-workspace.yamltest/e2e-dev/first-party.test.tstest/unit/proxy-configs.test.tstest/unit/rewrite-ast.test.ts
💤 Files with no reviewable changes (1)
- packages/devtools-app/nuxt.config.ts
Fathom's bot detection uses the connecting source IP and ignores X-Forwarded-For from arbitrary servers. Proxying beacons made every visitor appear to originate from the server's datacenter IP, so they were flagged as bots. Remove proxy capability from the Fathom registry entry. Bundling is still supported: the script is served from the user's origin (faster load, ad-blocker resistant) and beacons go directly to cdn.usefathom.com via the existing neutralize-domain-check patch. To support this, BundleCapability gains its own optional sdkPatches field (independent of proxy), and the neutralize-domain-check patch now carries an explicit `domain` instead of inferring it from proxy rewrites. The transform plugin skips API rewrites when no proxy is active so bundled-only scripts don't crash trying to call __nuxtScripts.* with no intercept plugin loaded. Resolves #720
Partytown's auto-configured resolveUrl routes all cross-origin requests through the proxy prefix. With partytown opted-in, Fathom beacons would get proxied via /_scripts/p/cdn.usefathom.com/* and trigger the bot detection failure this PR is meant to fix. Remove partytown from the Fathom registry entry and its matching forwards config.
harlan-zw
force-pushed
the
fix/fathom-bundle-only
branch
from
aa5d0a2 to
8fbf0db
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/script/src/plugins/transform.ts (1)
469-489:⚠️ Potential issue | 🔴 CriticalCritical: bundle-only
sdkPatchesnever applied — gated onproxyRewrites?.lengthwhich is undefined for Fathom.
sdkPatchesis correctly derived frombundleConfig?.sdkPatchesat line 473 and passed todownloadScript, but the only production call torewriteScriptUrlsAST(line 166) is gated onproxyRewrites?.length:if (proxyRewrites?.length && res) { const rewritten = rewriteScriptUrlsAST(content, filename, proxyRewrites, sdkPatches, ...) }For Fathom (bundle-only, no proxy),
proxyConfigis undefined →proxyRewritesis undefined → gate evaluates to false →rewriteScriptUrlsASTis never called → theneutralize-domain-checkpatch is never applied.Unit tests pass because they invoke
rewriteScriptUrlsASTdirectly (e.g., line 251:rewriteScriptUrlsAST(code, 'fathom.js', [], fathomPatches)), bypassing the gate entirely.🔧 Suggested fix
- if (proxyRewrites?.length && res) { + if ((proxyRewrites?.length || sdkPatches?.length) && res) { const content = res.toString('utf-8') - const rewritten = rewriteScriptUrlsAST(content, filename, proxyRewrites, sdkPatches, { skipApiRewrites, neutralizeCanvas }) + const rewritten = rewriteScriptUrlsAST(content, filename, proxyRewrites ?? [], sdkPatches, { skipApiRewrites, neutralizeCanvas }) res = Buffer.from(rewritten, 'utf-8') - logger.debug(`Rewrote ${proxyRewrites.length} URL patterns in ${filename}`) + logger.debug(`Rewrote ${proxyRewrites?.length ?? 0} URL patterns / ${sdkPatches?.length ?? 0} SDK patches in ${filename}`) }Also consider hashing
sdkPatchesin the cache key (currently only hashesproxyRewrites), otherwise bundle cache won't invalidate whenbundle.sdkPatcheschanges.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/plugins/transform.ts` around lines 469 - 489, The bundle-only sdkPatches (derived into variable sdkPatches) are never applied because the call to rewriteScriptUrlsAST is gated on proxyRewrites?.length; for bundle-only scripts proxyRewrites is undefined so rewriteScriptUrlsAST is skipped. Fix by ensuring rewriteScriptUrlsAST is invoked whenever sdkPatches is provided (or when bundleConfig exists), e.g. change the gate around rewriteScriptUrlsAST (the logic that currently checks proxyRewrites?.length && res) to also run when sdkPatches is non-empty; update the path that calls downloadScript/rewriteScriptUrlsAST (referencing functions/vars: sdkPatches, proxyRewrites, rewriteScriptUrlsAST, downloadScript) and include sdkPatches in any cache key/hash computation so bundle cache invalidates when bundle.sdkPatches changes.
🧹 Nitpick comments (1)
test/e2e-dev/first-party.test.ts (1)
651-912: Consider adding an integration assertion that Fathom's bundle is actually patched.Removing Fathom from the proxy provider sets and deleting the dedicated e2e are correct given the behavior shift. However, the remaining coverage only verifies that some bundled script is loaded from
/_scripts/assets/; nothing asserts that theneutralize-domain-checkpatch was applied to the Fathom bundle. Given the build-time gate issue flagged intransform.ts, this suite currently passes while the fix is silently a no-op.A minimal guard in the
bundled script integrityblock would catch this regression:💡 Suggested addition
it('fathom bundle has neutralize-domain-check applied', () => { const cacheDir = join(fixtureDir, 'node_modules/.cache/nuxt/scripts') // find the cached bundle that originated from cdn.usefathom.com/script.js const fathomFile = readdirSync(cacheDir, { recursive: true }) .filter((f): f is string => typeof f === 'string' && f.endsWith('.js')) .map(f => join(cacheDir, f)) .find(p => readFileSync(p, 'utf-8').includes('usefathom')) expect(fathomFile, 'Fathom bundle not found in cache').toBeDefined() const content = readFileSync(fathomFile!, 'utf-8') // The neutralize-domain-check patch rewrites `< 0` to `< -1` on the indexOf comparison expect(content).not.toMatch(/indexOf\(["']cdn\.usefathom\.com["']\)\s*<\s*0/) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e-dev/first-party.test.ts` around lines 651 - 912, Add a new assertion in the "bundled script integrity" tests that searches the Nuxt scripts cache (using fixtureDir and the same cache lookup logic in other tests: readdirSync + readFileSync) for the Fathom bundle (string match 'usefathom' or the cdn host) and assert the neutralize-domain-check patch from transform.ts was applied by verifying the indexOf comparison was rewritten (e.g. ensure the file content does not match /indexOf\(["']cdn\.usefathom\.com["']\)\s*<\s*0/ or checks for the expected patched token like '< -1'); name the test something like 'fathom bundle has neutralize-domain-check applied' and fail with a clear message if the cached bundle is missing or the patch is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/script/src/registry.ts`:
- Around line 345-351: The AST rewrite is being skipped because downloadScript
currently gates execution on proxyRewrites?.length; update the gate in the
downloadScript function to also allow when bundle.sdkPatches is present (e.g.,
check proxyRewrites?.length || bundle?.sdkPatches?.length or a boolean for
bundle.sdkPatches) so that sdkPatches like { type: 'neutralize-domain-check',
domain: 'cdn.usefathom.com' } are processed; keep the existing type-guard/filter
in rewrite-ast.ts unchanged and ensure the code references the bundle and
sdkPatches symbols and the neutralize-domain-check patch type so the patch is
applied for bundled Fathom scripts at build time.
---
Outside diff comments:
In `@packages/script/src/plugins/transform.ts`:
- Around line 469-489: The bundle-only sdkPatches (derived into variable
sdkPatches) are never applied because the call to rewriteScriptUrlsAST is gated
on proxyRewrites?.length; for bundle-only scripts proxyRewrites is undefined so
rewriteScriptUrlsAST is skipped. Fix by ensuring rewriteScriptUrlsAST is invoked
whenever sdkPatches is provided (or when bundleConfig exists), e.g. change the
gate around rewriteScriptUrlsAST (the logic that currently checks
proxyRewrites?.length && res) to also run when sdkPatches is non-empty; update
the path that calls downloadScript/rewriteScriptUrlsAST (referencing
functions/vars: sdkPatches, proxyRewrites, rewriteScriptUrlsAST, downloadScript)
and include sdkPatches in any cache key/hash computation so bundle cache
invalidates when bundle.sdkPatches changes.
---
Nitpick comments:
In `@test/e2e-dev/first-party.test.ts`:
- Around line 651-912: Add a new assertion in the "bundled script integrity"
tests that searches the Nuxt scripts cache (using fixtureDir and the same cache
lookup logic in other tests: readdirSync + readFileSync) for the Fathom bundle
(string match 'usefathom' or the cdn host) and assert the
neutralize-domain-check patch from transform.ts was applied by verifying the
indexOf comparison was rewritten (e.g. ensure the file content does not match
/indexOf\(["']cdn\.usefathom\.com["']\)\s*<\s*0/ or checks for the expected
patched token like '< -1'); name the test something like 'fathom bundle has
neutralize-domain-check applied' and fail with a clear message if the cached
bundle is missing or the patch is absent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 708f3fa5-363d-45e5-a6ed-bf7d19ccf80e
📒 Files selected for processing (8)
docs/content/scripts/fathom-analytics.mdpackages/script/src/plugins/rewrite-ast.tspackages/script/src/plugins/transform.tspackages/script/src/registry.tspackages/script/src/runtime/types.tstest/e2e-dev/first-party.test.tstest/unit/proxy-configs.test.tstest/unit/rewrite-ast.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/content/scripts/fathom-analytics.md
- test/unit/proxy-configs.test.ts
- packages/script/src/plugins/rewrite-ast.ts
downloadScript gated the AST rewrite on proxyRewrites.length, so bundle-only scripts (Fathom) with sdkPatches but no proxy config skipped the rewrite entirely. The neutralize-domain-check patch never ran at build time, defeating the bundle path's purpose. Run the rewrite when either proxyRewrites or sdkPatches is present, and include sdkPatches in the cache key so patched/unpatched bundles don't collide.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/script/src/plugins/transform.ts (1)
141-144: Cache key prefixbundle-proxy:is now a misnomer for bundle-only sdkPatches.With
hasRewritesalso true when onlysdkPatchesare present (no proxy), entries like Fathom's neutralize-domain-check bundle land under abundle-proxy:key even though no proxy is involved. Functionally fine, but the name will mislead future debugging. Consider renaming to something neutral likebundle-patched:(orbundle-rewritten:).♻️ Suggested rename
- const cacheKey = hasRewrites ? `bundle-proxy:${filename.replace('.js', `${rewriteHash}.js`)}` : `bundle:${filename}` + const cacheKey = hasRewrites ? `bundle-patched:${filename.replace('.js', `${rewriteHash}.js`)}` : `bundle:${filename}`Note that this will invalidate existing
bundle-proxy:*cache entries on upgrade, which is arguably desirable given the behavior change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/plugins/transform.ts` around lines 141 - 144, The cache key prefix `bundle-proxy:` is misleading when only sdkPatches are present because hasRewrites is true for sdkPatches alone; update the cache key logic in transform.ts (symbols: hasRewrites, proxyRewrites, sdkPatches, rewriteHash, cacheKey, filename) to use a neutral prefix such as `bundle-patched:` (or `bundle-rewritten:`) instead of `bundle-proxy:` when hasRewrites is true, and adjust the surrounding comment to reflect that the key covers both proxy rewrites and bundle-only patches; ensure the conditional that builds cacheKey still uses rewriteHash when hasRewrites is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/script/src/plugins/transform.ts`:
- Around line 472-476: Add a clarifying comment explaining that the resolution
of sdkPatches prefers proxyConfig.sdkPatches over bundleConfig.sdkPatches (via
the nullish coalescing on sdkPatches) is intentional; update the comment around
the bundleConfig and sdkPatches lines (referencing script.bundle, bundleConfig,
proxyConfig and sdkPatches) to state explicitly that when a proxy is configured
its sdkPatches take precedence and will override any bundle-level sdkPatches by
design to enforce proxy/bundle separation.
---
Nitpick comments:
In `@packages/script/src/plugins/transform.ts`:
- Around line 141-144: The cache key prefix `bundle-proxy:` is misleading when
only sdkPatches are present because hasRewrites is true for sdkPatches alone;
update the cache key logic in transform.ts (symbols: hasRewrites, proxyRewrites,
sdkPatches, rewriteHash, cacheKey, filename) to use a neutral prefix such as
`bundle-patched:` (or `bundle-rewritten:`) instead of `bundle-proxy:` when
hasRewrites is true, and adjust the surrounding comment to reflect that the key
covers both proxy rewrites and bundle-only patches; ensure the conditional that
builds cacheKey still uses rewriteHash when hasRewrites is true.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc6ea92a-5094-41c4-96de-2678745e1755
📒 Files selected for processing (1)
packages/script/src/plugins/transform.ts
Add an integration test that runs the full transformer → downloadScript → rewriteScriptUrlsAST pipeline with a Fathom-like stub and asserts the neutralize-domain-check patch actually lands in the stored bundle. The earlier regression silently no-op'd because direct unit tests of rewriteScriptUrlsAST bypassed the build-time gate. Also rename the patched-bundle cache key from bundle-proxy: to bundle-patched: now that it covers bundle-only sdkPatches (no proxy), and document the proxy-over-bundle sdkPatches precedence.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/unit/bundle-sdk-patches.test.ts`:
- Around line 22-34: The tests declare mockBundleStorage and fetchMock at
top-level and then reference them inside a vi.mock factory, which runs during
hoisting; fix by moving shared mock state into vi.hoisted() so the factory can
access initialized mocks: create hoisted variables for mockBundleStorage and
fetchMock via vi.hoisted(), initialize their mock methods there, then change the
vi.mock('../../packages/script/src/assets', ...) factory to return the hoisted
mockBundleStorage and use vi.stubGlobal('fetch', fetchMock) after hoisting;
apply the same change to test/unit/transform.test.ts and
test/unit/bundle-two-deploy-repro.test.ts.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcb7a234-73a9-409d-91fa-0b6056af22df
📒 Files selected for processing (2)
packages/script/src/plugins/transform.tstest/unit/bundle-sdk-patches.test.ts
| const mockBundleStorage: any = { | ||
| getItem: vi.fn(), | ||
| setItem: vi.fn(), | ||
| getItemRaw: vi.fn(), | ||
| setItemRaw: vi.fn(), | ||
| hasItem: vi.fn().mockResolvedValue(false), | ||
| } | ||
| vi.mock('../../packages/script/src/assets', () => ({ | ||
| bundleStorage: vi.fn(() => mockBundleStorage), | ||
| })) | ||
|
|
||
| const fetchMock = vi.fn() | ||
| vi.stubGlobal('fetch', fetchMock) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect vi.mock factories in this test and nearby unit tests for top-level captures.
rg -n -C4 "vi\.mock\(" --type=tsRepository: nuxt/scripts
Length of output: 13278
🏁 Script executed:
#!/bin/bash
# Check if the test files with similar patterns exist and are part of the test suite
ls -lh test/unit/transform.test.ts test/unit/bundle-two-deploy-repro.test.ts test/unit/bundle-sdk-patches.test.ts 2>/dev/null
echo "---"
# Look for vi.hoisted usage patterns in the codebase
rg "vi\.hoisted\(" --type=ts -nRepository: nuxt/scripts
Length of output: 397
🌐 Web query:
Vitest vi.mock hoisting behavior temporal dead zone TDZ factory function
💡 Result:
Vitest's vi.mock calls are hoisted to the top of the test file and executed before any imports or top-level code, similar to JavaScript hoisting but enforced by Vitest's static analysis and transformation. This hoisting ensures mocks are in place before modules are loaded. The factory function in vi.mock(path, factory) runs in this hoisted position, creating a Temporal Dead Zone (TDZ)-like restriction: it cannot access let/const variables declared later in the file, as they are uninitialized (ReferenceError). Top-level var variables might access as undefined, but this is unreliable. Vitest throws an explicit error if top-level variables appear in the factory: "There was an error when mocking a module. If you are using 'vi.mock' factory, make sure there are no top level variables inside, since this call is hoisted to top of the file." Workarounds: - Use vi.hoisted( => {...}) to define shared mocks/variables before vi.mock, accessible in the factory. - Use dynamic imports inside the factory (e.g., await import('./other-module')). - For non-hoisted mocking, use vi.doMock, but it only affects subsequent imports. - Avoid top-level variables in factories; define everything inside or use vi.hoisted. Example causing TDZ-like error: const myVar = 'value'; // let/const - TDZ when factory runs vi.mock('./module', => ({ fn: vi.fn( => myVar) })); // Error: myVar not initialized Correct with vi.hoisted: const mocks = vi.hoisted( => ({ myFn: vi.fn })); vi.mock('./module', => ({ fn: mocks.myFn })); Official docs confirm: "you cannot use any variables inside the factory that are defined outside the factory" (vitest.dev/api/vi.html#vi-mock).
Citations:
- 1: https://vitest.dev/api/vi.html
- 2: https://vitest.dev/guide/mocking
- 3: https://vitest.dev/guide/mocking/modules.html
- 4: Import order breaks tests when using vi.mock vitest-dev/vitest#6496
- 5: Unable to use local variable as argument for
vi.fn().mockReturnValue()vitest-dev/vitest#1381 - 6: https://stackoverflow.com/questions/78144648/error-vitest-there-was-an-error-when-mocking-a-module-if-you-are-using-vi-m
Move mock state into vi.hoisted() before using it in a vi.mock() factory.
Vitest forbids top-level variables in vi.mock() factories. The factory runs during module hoisting before mockBundleStorage and fetchMock are initialized, causing a runtime error. Use vi.hoisted() to define shared mock state that is accessible to the factory.
🧪 Proposed fix
-const mockBundleStorage: any = {
- getItem: vi.fn(),
- setItem: vi.fn(),
- getItemRaw: vi.fn(),
- setItemRaw: vi.fn(),
- hasItem: vi.fn().mockResolvedValue(false),
-}
+const { mockBundleStorage, fetchMock } = vi.hoisted(() => ({
+ mockBundleStorage: {
+ getItem: vi.fn(),
+ setItem: vi.fn(),
+ getItemRaw: vi.fn(),
+ setItemRaw: vi.fn(),
+ hasItem: vi.fn().mockResolvedValue(false),
+ },
+ fetchMock: vi.fn(),
+}))
+
vi.mock('../../packages/script/src/assets', () => ({
bundleStorage: vi.fn(() => mockBundleStorage),
}))
-const fetchMock = vi.fn()
vi.stubGlobal('fetch', fetchMock)Note: test/unit/transform.test.ts and test/unit/bundle-two-deploy-repro.test.ts have the same issue and should also be fixed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mockBundleStorage: any = { | |
| getItem: vi.fn(), | |
| setItem: vi.fn(), | |
| getItemRaw: vi.fn(), | |
| setItemRaw: vi.fn(), | |
| hasItem: vi.fn().mockResolvedValue(false), | |
| } | |
| vi.mock('../../packages/script/src/assets', () => ({ | |
| bundleStorage: vi.fn(() => mockBundleStorage), | |
| })) | |
| const fetchMock = vi.fn() | |
| vi.stubGlobal('fetch', fetchMock) | |
| const { mockBundleStorage, fetchMock } = vi.hoisted(() => ({ | |
| mockBundleStorage: { | |
| getItem: vi.fn(), | |
| setItem: vi.fn(), | |
| getItemRaw: vi.fn(), | |
| setItemRaw: vi.fn(), | |
| hasItem: vi.fn().mockResolvedValue(false), | |
| }, | |
| fetchMock: vi.fn(), | |
| })) | |
| vi.mock('../../packages/script/src/assets', () => ({ | |
| bundleStorage: vi.fn(() => mockBundleStorage), | |
| })) | |
| vi.stubGlobal('fetch', fetchMock) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/bundle-sdk-patches.test.ts` around lines 22 - 34, The tests declare
mockBundleStorage and fetchMock at top-level and then reference them inside a
vi.mock factory, which runs during hoisting; fix by moving shared mock state
into vi.hoisted() so the factory can access initialized mocks: create hoisted
variables for mockBundleStorage and fetchMock via vi.hoisted(), initialize their
mock methods there, then change the vi.mock('../../packages/script/src/assets',
...) factory to return the hoisted mockBundleStorage and use
vi.stubGlobal('fetch', fetchMock) after hoisting; apply the same change to
test/unit/transform.test.ts and test/unit/bundle-two-deploy-repro.test.ts.
1 participant
🔗 Linked issue
Resolves #720
❓ Type of change
📚 Description
Fathom's bot detection uses the connecting source IP and ignores
X-Forwarded-Forfrom arbitrary servers. Proxying beacons through the Nuxt server made every visitor appear to originate from the datacenter IP, so Fathom flagged them all as bots.This drops the
proxycapability from the Fathom registry entry. Bundling is still supported: the script is served from the user's origin (faster load, ad-blocker resistant for domain-based blocking), and beacons go directly tocdn.usefathom.comfrom the browser via the existingneutralize-domain-checkpatch, so real client IPs reach Fathom.To enable bundle-without-proxy for scripts that need SDK patching,
BundleCapabilitygains its ownsdkPatchesfield, andneutralize-domain-checknow carries an explicitdomain(instead of inferring it from proxy rewrites). The transform plugin also skips__nuxtScripts.*API rewrites when no proxy is active so bundled-only scripts don't crash with no intercept plugin loaded.Also includes a small chore commit bumping a few catalog deps and swapping
@vueuse/nuxtfor@vueuse/coreindevtools-app(the module wrapper isn't needed there).