From ed92695253a356234d24f5e428790f108924d5ef Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 28 May 2025 21:14:17 -0500 Subject: [PATCH 1/4] feat: failure detection --- .../src/__tests__/loadClerkJsScript.test.ts | 132 ++++++++++++++---- packages/shared/src/loadClerkJsScript.ts | 78 +++++++++-- 2 files changed, 165 insertions(+), 45 deletions(-) diff --git a/packages/shared/src/__tests__/loadClerkJsScript.test.ts b/packages/shared/src/__tests__/loadClerkJsScript.test.ts index 2e259e10292..ee365188b8a 100644 --- a/packages/shared/src/__tests__/loadClerkJsScript.test.ts +++ b/packages/shared/src/__tests__/loadClerkJsScript.test.ts @@ -4,11 +4,8 @@ import { loadClerkJsScript, setClerkJsLoadingErrorPackageName, } from '../loadClerkJsScript'; -import { loadScript } from '../loadScript'; import { getMajorVersion } from '../versionSelector'; -jest.mock('../loadScript'); - setClerkJsLoadingErrorPackageName('@clerk/clerk-react'); const jsPackageMajorVersion = getMajorVersion(JS_PACKAGE_VERSION); @@ -17,8 +14,12 @@ describe('loadClerkJsScript(options)', () => { beforeEach(() => { jest.clearAllMocks(); - (loadScript as jest.Mock).mockResolvedValue(undefined); document.querySelector = jest.fn().mockReturnValue(null); + // Mock document.body for script creation + Object.defineProperty(document, 'body', { + value: document.createElement('body'), + writable: true, + }); }); test('throws error when publishableKey is missing', async () => { @@ -28,48 +29,111 @@ describe('loadClerkJsScript(options)', () => { }); test('loads script when no existing script is found', async () => { - await loadClerkJsScript({ publishableKey: mockPublishableKey }); - - expect(loadScript).toHaveBeenCalledWith( - expect.stringContaining( - `https://foo-bar-13.clerk.accounts.dev/npm/@clerk/clerk-js@${jsPackageMajorVersion}/dist/clerk.browser.js`, - ), - expect.objectContaining({ - async: true, - crossOrigin: 'anonymous', - beforeLoad: expect.any(Function), - }), + const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); + + // Find the script that was added to the DOM + const addedScript = document.body.querySelector('script[data-clerk-js-script]') as HTMLScriptElement; + expect(addedScript).toBeTruthy(); + expect(addedScript.src).toContain( + `https://foo-bar-13.clerk.accounts.dev/npm/@clerk/clerk-js@${jsPackageMajorVersion}/dist/clerk.browser.js`, ); + expect(addedScript.async).toBe(true); + expect(addedScript.getAttribute('crossorigin')).toBe('anonymous'); + + // Simulate successful load + addedScript.dispatchEvent(new Event('load')); + + const result = await loadPromise; + expect(result).toBe(addedScript); + expect(addedScript.getAttribute('data-clerk-loaded')).toBe('true'); + }); + + test('resolves immediately when script is already loaded', async () => { + const mockExistingScript = document.createElement('script'); + mockExistingScript.setAttribute('data-clerk-loaded', 'true'); + document.querySelector = jest.fn().mockReturnValue(mockExistingScript); + + const result = await loadClerkJsScript({ publishableKey: mockPublishableKey }); + + expect(result).toBe(mockExistingScript); + // Should not create a new script + expect(document.body.querySelector('script[data-clerk-js-script]')).toBeNull(); }); - test('uses existing script when found', async () => { + test('waits for existing loading script to complete', async () => { const mockExistingScript = document.createElement('script'); + // Script exists but is not marked as loaded yet document.querySelector = jest.fn().mockReturnValue(mockExistingScript); const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); - mockExistingScript.dispatchEvent(new Event('load')); - await expect(loadPromise).resolves.toBe(mockExistingScript); - expect(loadScript).not.toHaveBeenCalled(); + // Simulate the existing script finishing loading + setTimeout(() => { + mockExistingScript.dispatchEvent(new Event('load')); + }, 10); + + const result = await loadPromise; + expect(result).toBe(mockExistingScript); + expect(mockExistingScript.getAttribute('data-clerk-loaded')).toBe('true'); }); - test('rejects when existing script fails to load', async () => { + test('removes failed script and rejects when existing script fails to load', async () => { const mockExistingScript = document.createElement('script'); document.querySelector = jest.fn().mockReturnValue(mockExistingScript); + const removeSpy = jest.spyOn(mockExistingScript, 'remove'); const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); mockExistingScript.dispatchEvent(new Event('error')); - await expect(loadPromise).rejects.toBe('Clerk: Failed to load Clerk'); - expect(loadScript).not.toHaveBeenCalled(); + await expect(loadPromise).rejects.toThrow('Clerk: Failed to load Clerk'); + expect(removeSpy).toHaveBeenCalled(); }); - test('throws error when loadScript fails', async () => { - (loadScript as jest.Mock).mockRejectedValue(new Error('Script load failed')); + test('handles race condition when script loads while setting up listeners', async () => { + const mockExistingScript = document.createElement('script'); + document.querySelector = jest.fn().mockReturnValue(mockExistingScript); - await expect(loadClerkJsScript({ publishableKey: mockPublishableKey })).rejects.toThrow( - 'Clerk: Failed to load Clerk', - ); + // Simulate script loading between the initial check and listener setup + setTimeout(() => { + mockExistingScript.setAttribute('data-clerk-loaded', 'true'); + }, 1); + + const result = await loadClerkJsScript({ publishableKey: mockPublishableKey }); + expect(result).toBe(mockExistingScript); + }); + + test('creates script with correct attributes', async () => { + const options = { + publishableKey: mockPublishableKey, + proxyUrl: 'https://proxy.clerk.com', + domain: 'custom.com', + nonce: 'test-nonce', + }; + + const loadPromise = loadClerkJsScript(options); + + const addedScript = document.body.querySelector('script[data-clerk-js-script]') as HTMLScriptElement; + expect(addedScript.getAttribute('data-clerk-publishable-key')).toBe(mockPublishableKey); + expect(addedScript.getAttribute('data-clerk-proxy-url')).toBe('https://proxy.clerk.com'); + expect(addedScript.getAttribute('data-clerk-domain')).toBe('custom.com'); + expect(addedScript.nonce).toBe('test-nonce'); + + // Complete the load + addedScript.dispatchEvent(new Event('load')); + await loadPromise; + }); + + test('rejects and removes script when new script fails to load', async () => { + const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); + + const addedScript = document.body.querySelector('script[data-clerk-js-script]') as HTMLScriptElement; + const removeSpy = jest.spyOn(addedScript, 'remove'); + + // Simulate script load failure + addedScript.dispatchEvent(new Event('error')); + + await expect(loadPromise).rejects.toThrow('Clerk: Failed to load Clerk'); + expect(removeSpy).toHaveBeenCalled(); }); }); @@ -120,6 +184,7 @@ describe('buildClerkJsScriptAttributes()', () => { 'all options', { publishableKey: mockPublishableKey, proxyUrl: mockProxyUrl, domain: mockDomain }, { + 'data-clerk-js-script': '', 'data-clerk-publishable-key': mockPublishableKey, 'data-clerk-proxy-url': mockProxyUrl, 'data-clerk-domain': mockDomain, @@ -128,14 +193,21 @@ describe('buildClerkJsScriptAttributes()', () => { [ 'only publishableKey', { publishableKey: mockPublishableKey }, - { 'data-clerk-publishable-key': mockPublishableKey }, + { + 'data-clerk-js-script': '', + 'data-clerk-publishable-key': mockPublishableKey, + }, ], [ 'publishableKey and proxyUrl', { publishableKey: mockPublishableKey, proxyUrl: mockProxyUrl }, - { 'data-clerk-publishable-key': mockPublishableKey, 'data-clerk-proxy-url': mockProxyUrl }, + { + 'data-clerk-js-script': '', + 'data-clerk-publishable-key': mockPublishableKey, + 'data-clerk-proxy-url': mockProxyUrl, + }, ], - ['no options', {}, {}], + ['no options', {}, { 'data-clerk-js-script': '' }], ])('returns correct attributes with %s', (_, input, expected) => { // @ts-ignore input loses correct type because of empty object expect(buildClerkJsScriptAttributes(input)).toEqual(expected); diff --git a/packages/shared/src/loadClerkJsScript.ts b/packages/shared/src/loadClerkJsScript.ts index 97647e3f0bd..ee3df44f089 100644 --- a/packages/shared/src/loadClerkJsScript.ts +++ b/packages/shared/src/loadClerkJsScript.ts @@ -2,7 +2,6 @@ import type { ClerkOptions, SDKMetadata, Without } from '@clerk/types'; import { buildErrorThrower } from './error'; import { createDevOrStagingUrlCache, parsePublishableKey } from './keys'; -import { loadScript } from './loadScript'; import { isValidProxyUrl, proxyUrlToAbsoluteURL } from './proxy'; import { addClerkPrefix } from './url'; import { versionSelector } from './versionSelector'; @@ -37,9 +36,11 @@ type LoadClerkJsScriptOptions = Without & { /** * Hotloads the Clerk JS script. * - * Checks for an existing Clerk JS script. If found, it returns a promise - * that resolves when the script loads. If not found, it uses the provided options to - * build the Clerk JS script URL and load the script. + * Checks for an existing Clerk JS script and handles various loading states: + * - If script already loaded successfully: resolves immediately + * - If script is currently loading: waits for completion + * - If script failed to load: attempts to load again + * - If no script exists: loads the script. * * @param opts - The options used to build the Clerk JS script URL and load the script. * Must include a `publishableKey` if no existing script is found. @@ -51,14 +52,35 @@ const loadClerkJsScript = async (opts?: LoadClerkJsScriptOptions) => { const existingScript = document.querySelector('script[data-clerk-js-script]'); if (existingScript) { + if (existingScript.getAttribute('data-clerk-loaded') === 'true') { + return Promise.resolve(existingScript); + } + return new Promise((resolve, reject) => { - existingScript.addEventListener('load', () => { + const handleLoad = () => { + cleanup(); + existingScript.setAttribute('data-clerk-loaded', 'true'); resolve(existingScript); - }); + }; + + const handleError = () => { + cleanup(); + existingScript.remove(); + reject(new Error(FAILED_TO_LOAD_ERROR)); + }; + + const cleanup = () => { + existingScript.removeEventListener('load', handleLoad); + existingScript.removeEventListener('error', handleError); + }; + + existingScript.addEventListener('load', handleLoad); + existingScript.addEventListener('error', handleError); - existingScript.addEventListener('error', () => { - reject(FAILED_TO_LOAD_ERROR); - }); + if (existingScript.getAttribute('data-clerk-loaded') === 'true') { + cleanup(); + resolve(existingScript); + } }); } @@ -67,11 +89,36 @@ const loadClerkJsScript = async (opts?: LoadClerkJsScriptOptions) => { return; } - return loadScript(clerkJsScriptUrl(opts), { - async: true, - crossOrigin: 'anonymous', - nonce: opts.nonce, - beforeLoad: applyClerkJsScriptAttributes(opts), + return new Promise((resolve, reject) => { + if (!document || !document.body) { + reject(new Error('loadClerkJsScript cannot be called when document does not exist')); + return; + } + + const script = document.createElement('script'); + script.async = true; + script.setAttribute('crossorigin', 'anonymous'); + if (opts.nonce) { + script.nonce = opts.nonce; + } + + const handleLoad = () => { + script.setAttribute('data-clerk-loaded', 'true'); + resolve(script); + }; + + const handleError = () => { + script.remove(); + reject(new Error(FAILED_TO_LOAD_ERROR)); + }; + + script.addEventListener('load', handleLoad); + script.addEventListener('error', handleError); + + applyClerkJsScriptAttributes(opts)(script); + + script.src = clerkJsScriptUrl(opts); + document.body.appendChild(script); }).catch(() => { throw new Error(FAILED_TO_LOAD_ERROR); }); @@ -81,7 +128,6 @@ const loadClerkJsScript = async (opts?: LoadClerkJsScriptOptions) => { * Generates a Clerk JS script URL. * * @param opts - The options to use when building the Clerk JS script URL. - * * @example * clerkJsScriptUrl({ publishableKey: 'pk_' }); */ @@ -112,6 +158,8 @@ const clerkJsScriptUrl = (opts: LoadClerkJsScriptOptions) => { const buildClerkJsScriptAttributes = (options: LoadClerkJsScriptOptions) => { const obj: Record = {}; + obj['data-clerk-js-script'] = ''; + if (options.publishableKey) { obj['data-clerk-publishable-key'] = options.publishableKey; } From 939985a902175de2471e802abc011b0b45eaa091 Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 29 May 2025 11:20:48 -0500 Subject: [PATCH 2/4] render blocking coordinator --- packages/nextjs/src/utils/clerk-js-script.tsx | 247 ++++++++++++++++-- .../shared/src/clerkJsBlockingCoordinator.ts | 246 +++++++++++++++++ packages/shared/src/index.ts | 1 + packages/shared/src/loadClerkJsScript.ts | 133 ++++------ 4 files changed, 527 insertions(+), 100 deletions(-) create mode 100644 packages/shared/src/clerkJsBlockingCoordinator.ts diff --git a/packages/nextjs/src/utils/clerk-js-script.tsx b/packages/nextjs/src/utils/clerk-js-script.tsx index cc39f3534e6..1d56724c416 100644 --- a/packages/nextjs/src/utils/clerk-js-script.tsx +++ b/packages/nextjs/src/utils/clerk-js-script.tsx @@ -1,20 +1,167 @@ import { useClerk } from '@clerk/clerk-react'; import { buildClerkJsScriptAttributes, clerkJsScriptUrl } from '@clerk/clerk-react/internal'; import NextScript from 'next/script'; -import React from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { useClerkNextOptions } from '../client-boundary/NextOptionsContext'; +// TODO: This will work once the exports are properly set up +// For now, we'll use the blocking coordinator pattern with inline script + +type LoadingState = 'idle' | 'loading' | 'loaded' | 'error'; + type ClerkJSScriptProps = { router: 'app' | 'pages'; + onLoad?: () => void; + onError?: (error: Error) => void; + onLoadingStateChange?: (state: LoadingState) => void; }; +// Inline blocking coordinator script (until we can import properly) +function getBlockingCoordinatorScript(): string { + return ` +(function() { + 'use strict'; + + if (window.__clerkJSBlockingCoordinator) { + return; + } + + var coordinator = { + state: 'idle', + scriptUrl: null, + scriptElement: null, + error: null, + callbacks: [], + + shouldAllowScript: function(scriptElement) { + if (!scriptElement.hasAttribute('data-clerk-js-script')) { + return true; + } + + if (this.scriptElement && this.scriptElement.src === scriptElement.src) { + return false; + } + + if (window.Clerk) { + this.setState('loaded'); + return false; + } + + if (this.state === 'loading') { + return false; + } + + this.adoptScript(scriptElement); + return true; + }, + + adoptScript: function(scriptElement) { + this.scriptElement = scriptElement; + this.scriptUrl = scriptElement.src; + this.setState('loading'); + + var self = this; + + scriptElement.addEventListener('load', function() { + scriptElement.setAttribute('data-clerk-loaded', 'true'); + self.setState('loaded'); + }); + + scriptElement.addEventListener('error', function() { + self.setState('error', new Error('ClerkJS failed to load')); + }); + }, + + registerCallback: function(callback) { + this.callbacks.push(callback); + + if (callback.onStateChange) { + callback.onStateChange(this.state); + } + + if (this.state === 'loaded' && callback.onLoad) { + callback.onLoad(); + } else if (this.state === 'error' && callback.onError && this.error) { + callback.onError(this.error); + } + }, + + setState: function(newState, error) { + this.state = newState; + if (error) this.error = error; + + for (var i = 0; i < this.callbacks.length; i++) { + var callback = this.callbacks[i]; + try { + if (callback.onStateChange) { + callback.onStateChange(newState); + } + + if (newState === 'loaded' && callback.onLoad) { + callback.onLoad(); + } else if (newState === 'error' && callback.onError && error) { + callback.onError(error); + } + } catch (e) { + console.error('ClerkJS coordinator callback error:', e); + } + } + } + }; + + var originalAppendChild = Node.prototype.appendChild; + Node.prototype.appendChild = function(child) { + if (child.tagName === 'SCRIPT' && child.hasAttribute('data-clerk-js-script')) { + if (!coordinator.shouldAllowScript(child)) { + return child; + } + } + + return originalAppendChild.call(this, child); + }; + + window.__clerkJSBlockingCoordinator = coordinator; +})(); + `.trim(); +} + +// Hook to get the current ClerkJS loading state from the blocking coordinator +function useClerkJSLoadingState() { + const [loadingState, setLoadingState] = useState('idle'); + + useEffect(() => { + if (typeof window === 'undefined') return; + + const coordinator = (window as any).__clerkJSBlockingCoordinator; + if (coordinator) { + coordinator.registerCallback({ + onStateChange: (state: LoadingState) => { + setLoadingState(state); + }, + }); + } + }, []); + + return { loadingState }; +} + +/** + * Enhanced ClerkJS Script component with bulletproof load detection. + * + * This component renders TWO script tags: + * 1. A render-blocking inline script that sets up the coordinator BEFORE any ClerkJS scripts + * 2. The actual ClerkJS script tag (which the coordinator will manage) + * + * The blocking script intercepts script creation and prevents duplicates at the browser level. + */ function ClerkJSScript(props: ClerkJSScriptProps) { const { publishableKey, clerkJSUrl, clerkJSVersion, clerkJSVariant, nonce } = useClerkNextOptions(); const { domain, proxyUrl } = useClerk(); + const scriptRef = useRef(null); /** - * If no publishable key, avoid appending an invalid script in the DOM. + * If no publishable key, avoid appending invalid scripts in the DOM. */ if (!publishableKey) { return null; @@ -31,26 +178,82 @@ function ClerkJSScript(props: ClerkJSScriptProps) { }; const scriptUrl = clerkJsScriptUrl(options); - /** - * Notes: - * `next/script` in 13.x.x when used with App Router will fail to pass any of our `data-*` attributes, resulting in errors - * Nextjs App Router will automatically move inline scripts inside `` - * Using the `nextjs/script` for App Router with the `beforeInteractive` strategy will throw an error because our custom script will be mounted outside the `html` tag. - */ - const Script = props.router === 'app' ? 'script' : NextScript; - - return ( -