From 46afdecd92ca38f32e84b9b0059e7e5e699acc50 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Fri, 6 Oct 2023 13:37:02 -0500 Subject: [PATCH 1/5] feat(nextjs): Support rendering ClerkProvider in client components --- packages/nextjs/src/app-beta/index.ts | 7 +++++++ .../src/app-router/client/ClerkProvider.tsx | 3 ++- .../src/client-boundary/ClerkProvider.tsx | 19 +++++++++++++++++++ packages/nextjs/src/components.client.ts | 2 +- 4 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 packages/nextjs/src/client-boundary/ClerkProvider.tsx diff --git a/packages/nextjs/src/app-beta/index.ts b/packages/nextjs/src/app-beta/index.ts index 760a069fe9..5251bbff1c 100644 --- a/packages/nextjs/src/app-beta/index.ts +++ b/packages/nextjs/src/app-beta/index.ts @@ -1,3 +1,10 @@ +import { deprecated } from '@clerk/shared'; + +deprecated( + '@clerk/nextjs/app-beta', + 'Use imports from `@clerk/nextjs` instead.\nFor more details, consult the middleware documentation: https://clerk.com/docs/nextjs/middleware', +); + export { auth } from './auth'; export { currentUser } from './currentUser'; export { ClerkProvider } from './ClerkProvider'; diff --git a/packages/nextjs/src/app-router/client/ClerkProvider.tsx b/packages/nextjs/src/app-router/client/ClerkProvider.tsx index 7bcdee4a91..8fe3ca193a 100644 --- a/packages/nextjs/src/app-router/client/ClerkProvider.tsx +++ b/packages/nextjs/src/app-router/client/ClerkProvider.tsx @@ -6,6 +6,7 @@ import React from 'react'; import { ClerkNextOptionsProvider } from '../../client-boundary/NextOptionsContext'; import { useSafeLayoutEffect } from '../../client-boundary/useSafeLayoutEffect'; import type { NextClerkProviderProps } from '../../types'; +import { mergeNextClerkPropsWithEnv } from '../../utils/mergeNextClerkPropsWithEnv'; import { useAwaitableNavigate } from './useAwaitableNavigate'; declare global { @@ -33,7 +34,7 @@ export const ClientClerkProvider = (props: NextClerkProviderProps) => { }; }, []); - const mergedProps = { ...props, navigate }; + const mergedProps = mergeNextClerkPropsWithEnv({ ...props, navigate }); return ( {/*// @ts-ignore*/} diff --git a/packages/nextjs/src/client-boundary/ClerkProvider.tsx b/packages/nextjs/src/client-boundary/ClerkProvider.tsx new file mode 100644 index 0000000000..3624aefd6e --- /dev/null +++ b/packages/nextjs/src/client-boundary/ClerkProvider.tsx @@ -0,0 +1,19 @@ +'use client'; + +import { useRouter } from 'next/compat/router'; +import React from 'react'; + +import { ClientClerkProvider } from '../app-router/client/ClerkProvider'; +import { ClerkProvider as PageClerkProvider } from '../pages/ClerkProvider'; +import { type NextClerkProviderProps } from '../types'; + +/** + * This is a compatibility layer to support a single ClerkProvider component in both the app and pages routers. + */ +export function ClerkProvider(props: NextClerkProviderProps) { + const router = useRouter(); + + const Provider = router ? PageClerkProvider : ClientClerkProvider; + + return ; +} diff --git a/packages/nextjs/src/components.client.ts b/packages/nextjs/src/components.client.ts index 6c41451e6d..2fcf82aa99 100644 --- a/packages/nextjs/src/components.client.ts +++ b/packages/nextjs/src/components.client.ts @@ -1,2 +1,2 @@ -export { ClerkProvider } from './pages/ClerkProvider'; +export { ClerkProvider } from './client-boundary/ClerkProvider'; export { SignedIn, SignedOut } from './client-boundary/controlComponents'; From 00624ac61a7272d8fac0adad48ac0f79bb70f267 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Fri, 6 Oct 2023 13:40:15 -0500 Subject: [PATCH 2/5] chore(nextjs): Remove unintended change --- packages/nextjs/src/app-beta/index.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/nextjs/src/app-beta/index.ts b/packages/nextjs/src/app-beta/index.ts index 5251bbff1c..760a069fe9 100644 --- a/packages/nextjs/src/app-beta/index.ts +++ b/packages/nextjs/src/app-beta/index.ts @@ -1,10 +1,3 @@ -import { deprecated } from '@clerk/shared'; - -deprecated( - '@clerk/nextjs/app-beta', - 'Use imports from `@clerk/nextjs` instead.\nFor more details, consult the middleware documentation: https://clerk.com/docs/nextjs/middleware', -); - export { auth } from './auth'; export { currentUser } from './currentUser'; export { ClerkProvider } from './ClerkProvider'; From e42e88261c6345b424d3c19dd6c0869c3e8ab5b6 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Fri, 6 Oct 2023 13:47:03 -0500 Subject: [PATCH 3/5] chore(repo): Add changeset --- .changeset/gorgeous-countries-appear.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gorgeous-countries-appear.md diff --git a/.changeset/gorgeous-countries-appear.md b/.changeset/gorgeous-countries-appear.md new file mode 100644 index 0000000000..9720c566af --- /dev/null +++ b/.changeset/gorgeous-countries-appear.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Update `` to work in client components within the app router. This allows rendering of the provider in client components, previously the pages router provider was being imported and throwing an error. From 9ebce0ae5c92d91c991dfa7342d5221476392b4f Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Tue, 17 Oct 2023 23:32:18 -0500 Subject: [PATCH 4/5] chore(repo): Add tests for next build --- integration/models/application.ts | 6 +-- integration/tests/global.teardown.ts | 12 +++-- integration/tests/next-build.test.ts | 72 ++++++++++++++++++++++++++++ package.json | 2 +- 4 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 integration/tests/next-build.test.ts diff --git a/integration/models/application.ts b/integration/models/application.ts index f2f080f30d..d5c50b30e0 100644 --- a/integration/models/application.ts +++ b/integration/models/application.ts @@ -68,9 +68,9 @@ export const application = (config: ApplicationConfig, appDirPath: string, appDi state.serverUrl = serverUrl; return { port, serverUrl, pid: proc.pid }; }, - build: async () => { - const log = logger.child({ prefix: 'build' }).info; - await run(scripts.build, { cwd: appDirPath, log }); + build: async ({ log }: { log?: (msg: string) => void } = {}) => { + const finalLog = log ?? logger.child({ prefix: 'build' }).info; + await run(scripts.build, { cwd: appDirPath, log: finalLog }); }, serve: async (opts: { port?: number; manualStart?: boolean } = {}) => { const port = opts.port || (await getPort()); diff --git a/integration/tests/global.teardown.ts b/integration/tests/global.teardown.ts index 19fde3e2f1..a2acdfddf2 100644 --- a/integration/tests/global.teardown.ts +++ b/integration/tests/global.teardown.ts @@ -12,11 +12,13 @@ setup('teardown long running apps', async () => { console.log('Skipping cleanup'); return; } - const runningAppsFoundInStateFile = Object.values(stateFile.getLongRunningApps()); - const matchingLongRunningApps = appConfigs.longRunningApps.getByPattern( - runningAppsFoundInStateFile.map(app => app.id), - ); - await Promise.all(matchingLongRunningApps.map(app => app.destroy())); + const runningAppsFoundInStateFile = Object.values(stateFile.getLongRunningApps() ?? {}); + if (runningAppsFoundInStateFile.length > 0) { + const matchingLongRunningApps = appConfigs.longRunningApps.getByPattern( + runningAppsFoundInStateFile.map(app => app.id), + ); + await Promise.all(matchingLongRunningApps.map(app => app.destroy())); + } stateFile.remove(); console.log('Long running apps destroyed'); }); diff --git a/integration/tests/next-build.test.ts b/integration/tests/next-build.test.ts new file mode 100644 index 0000000000..1c0a01d8ef --- /dev/null +++ b/integration/tests/next-build.test.ts @@ -0,0 +1,72 @@ +import { expect, test } from '@playwright/test'; + +import type { Application } from '../models/application'; +import { appConfigs } from '../presets'; + +test.describe('next build @nextjs', () => { + test.describe.configure({ mode: 'parallel' }); + let app: Application; + const output = []; + + test.beforeAll(async () => { + app = await appConfigs.next.appRouter + .clone() + .addFile( + 'src/app/provider.tsx', + () => `'use client' +import { ClerkProvider } from "@clerk/nextjs" + +export function Provider({ children }: { children: any }) { + return ( + + {children} + + ) +}`, + ) + .addFile( + 'src/app/layout.tsx', + () => `import './globals.css'; +import { Inter } from 'next/font/google'; +import { Provider } from './provider'; + +const inter = Inter({ subsets: ['latin'] }); + +export const metadata = { + title: 'Create Next App', + description: 'Generated by create next app', +}; + +export default function RootLayout({ children }: { children: React.ReactNode }) { + return ( + + + {children} + + + ); +} + `, + ) + .commit(); + await app.setup(); + await app.withEnv(appConfigs.envs.withEmailCodes); + await app.build({ log: msg => output.push(msg) }); + }); + + test.afterAll(async () => { + await app.teardown(); + }); + + test('When is used as a client component, builds successfully and does not force dynamic rendering', () => { + const dynamicIndicator = 'λ'; + + /** + * Using /_not-found as it is an internal page that should statically render by default. + * This is a good indicator of whether or not the entire app has been opted-in to dynamic rendering. + */ + const notFoundPageLine = output.find(msg => msg.includes('/_not-found')); + + expect(notFoundPageLine).not.toContain(dynamicIndicator); + }); +}); diff --git a/package.json b/package.json index 740c609add..2d59e85930 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,7 @@ "test:ci": "FORCE_COLOR=1 turbo test --concurrency=${TURBO_CONCURRENCY:-80%}", "test:integration:base": "DEBUG=1 npx playwright test --config integration/playwright.config.ts", "test:integration:generic": "E2E_APP_ID=react.vite.* npm run test:integration:base -- --grep @generic", - "test:integration:nextjs": "E2E_APP_ID=next.appRouter.withEmailCodes npm run test:integration:base -- --grep @generic", + "test:integration:nextjs": "E2E_APP_ID=next.appRouter.withEmailCodes npm run test:integration:base -- --grep \"@generic|@nextjs\"", "test:integration:remix": "echo 'placeholder'", "test:integration:deployment:nextjs": "DEBUG=1 npx playwright test --config integration/playwright.deployments.config.ts", "clean": "turbo clean", From 83f81fbaeba75518cf3a28d8f0198d1f02febe79 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Tue, 17 Oct 2023 23:41:45 -0500 Subject: [PATCH 5/5] chore(repo): Add buildOutput property on Application instead of accepting a custom log --- integration/models/application.ts | 16 +++++++++++++--- integration/tests/next-build.test.ts | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/integration/models/application.ts b/integration/models/application.ts index d5c50b30e0..992cfd92b0 100644 --- a/integration/models/application.ts +++ b/integration/models/application.ts @@ -16,6 +16,7 @@ export const application = (config: ApplicationConfig, appDirPath: string, appDi const now = Date.now(); const stdoutFilePath = path.resolve(appDirPath, `e2e.${now}.log`); const stderrFilePath = path.resolve(appDirPath, `e2e.${now}.err.log`); + let buildOutput = ''; const self = { name, @@ -68,9 +69,18 @@ export const application = (config: ApplicationConfig, appDirPath: string, appDi state.serverUrl = serverUrl; return { port, serverUrl, pid: proc.pid }; }, - build: async ({ log }: { log?: (msg: string) => void } = {}) => { - const finalLog = log ?? logger.child({ prefix: 'build' }).info; - await run(scripts.build, { cwd: appDirPath, log: finalLog }); + build: async () => { + const log = logger.child({ prefix: 'build' }).info; + await run(scripts.build, { + cwd: appDirPath, + log: (msg: string) => { + buildOutput += `\n${msg}`; + log(msg); + }, + }); + }, + get buildOutput() { + return buildOutput; }, serve: async (opts: { port?: number; manualStart?: boolean } = {}) => { const port = opts.port || (await getPort()); diff --git a/integration/tests/next-build.test.ts b/integration/tests/next-build.test.ts index 1c0a01d8ef..7f5cf9394e 100644 --- a/integration/tests/next-build.test.ts +++ b/integration/tests/next-build.test.ts @@ -51,7 +51,7 @@ export default function RootLayout({ children }: { children: React.ReactNode }) .commit(); await app.setup(); await app.withEnv(appConfigs.envs.withEmailCodes); - await app.build({ log: msg => output.push(msg) }); + await app.build(); }); test.afterAll(async () => { @@ -65,7 +65,7 @@ export default function RootLayout({ children }: { children: React.ReactNode }) * Using /_not-found as it is an internal page that should statically render by default. * This is a good indicator of whether or not the entire app has been opted-in to dynamic rendering. */ - const notFoundPageLine = output.find(msg => msg.includes('/_not-found')); + const notFoundPageLine = app.buildOutput.split('\n').find(msg => msg.includes('/_not-found')); expect(notFoundPageLine).not.toContain(dynamicIndicator); });