-
Notifications
You must be signed in to change notification settings - Fork 87
fix(fathom-analytics): drop proxy support, bundle-only with sdkPatches #722
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
Changes from all commits
567f95e
8fbf0db
156e583
c187072
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,118 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Integration guard for bundle-only sdkPatches (nuxt/scripts#720 / Fathom). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Exercises the full NuxtScriptBundleTransformer β downloadScript β | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // rewriteScriptUrlsAST pipeline to prove the neutralize-domain-check patch is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // actually applied to the stored bundle. A prior regression gated the rewrite | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // on proxyRewrites.length, so bundle-only patches were silently dropped while | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // direct unit tests of rewriteScriptUrlsAST still passed. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { AssetBundlerTransformerOptions } from '../../packages/script/src/plugins/transform' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { hash } from 'ohash' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { hasProtocol } from 'ufo' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { describe, expect, it, vi } from 'vitest' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { NuxtScriptBundleTransformer } from '../../packages/script/src/plugins/transform' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vi.mock('ohash', async (og) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mod = await og<typeof import('ohash')>() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { ...mod, hash: vi.fn(mod.hash) } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vi.mock('ufo', async (og) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mod = await og<typeof import('ufo')>() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { ...mod, hasProtocol: vi.fn(mod.hasProtocol) } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π§© 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:
π‘ 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:
Move mock state into Vitest forbids top-level variables in π§ͺ 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: π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vi.mock('@nuxt/kit', async (og) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mod = await og<typeof import('@nuxt/kit')>() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nuxt = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options: { buildDir: '.nuxt', app: { baseURL: '/' }, runtimeConfig: { app: {} } }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hooks: { hook: vi.fn() }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { ...mod, useNuxt: () => nuxt, tryUseNuxt: () => nuxt } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vi.mocked(hasProtocol).mockImplementation(() => true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vi.mocked(hash).mockImplementation(() => 'fathom-script') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function mockUpstream(bytes: Buffer) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fetchMock.mockResolvedValueOnce({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ok: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| arrayBuffer: () => Promise.resolve(bytes), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { get: () => null }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _data: bytes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } as any) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function runTransform(code: string, options: AssetBundlerTransformerOptions) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const plugin = NuxtScriptBundleTransformer(options).vite() as any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await plugin.transform.handler.call({}, code, 'file.js') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('bundle-only sdkPatches integration', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fathomLike = Buffer.from( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `(function(){var e=document.currentScript;if(e.src.indexOf("cdn.usefathom.com")<0){t="custom"}})();`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('applies neutralize-domain-check to bundle-only scripts (no proxy)', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mockUpstream(fathomLike) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const renderedScript = new Map() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await runTransform( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `const instance = useScriptFathomAnalytics({ site: '123' }, { bundle: true })`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| renderedScript, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scripts: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bundle: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolve: () => 'https://cdn.usefathom.com/script.js', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sdkPatches: [{ type: 'neutralize-domain-check', domain: 'cdn.usefathom.com' }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import: { name: 'useScriptFathomAnalytics', from: '' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const stored = [...renderedScript.values()][0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(stored, 'bundle was not stored').toBeDefined() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const content = (stored.content as Buffer).toString('utf-8') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Patch rewrites `< 0` to `< -1` on the fathom domain indexOf comparison, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // preserving the original whitespace (minified `<0` stays minified). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(content).toMatch(/indexOf\("cdn\.usefathom\.com"\)\s*<\s*-1/) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(content).not.toMatch(/indexOf\("cdn\.usefathom\.com"\)\s*<\s*0\b/) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('leaves bundles untouched when no patches are configured', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mockUpstream(fathomLike) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const renderedScript = new Map() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await runTransform( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `const instance = useScript('https://cdn.usefathom.com/script.js', { bundle: true })`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| renderedScript, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scripts: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bundle: { resolve: () => 'https://cdn.usefathom.com/script.js' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import: { name: 'useScript', from: '' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const stored = [...renderedScript.values()][0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(stored).toBeDefined() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const content = (stored.content as Buffer).toString('utf-8') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(content).toBe(fathomLike.toString('utf-8')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.