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 760a069fe9a..5251bbff1cf 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 7bcdee4a91a..8fe3ca193ab 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 00000000000..3624aefd6e1 --- /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 6c41451e6d7..2fcf82aa99c 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 5251bbff1cf..760a069fe9a 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 00000000000..9720c566af9 --- /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 f2f080f30d0..d5c50b30e0c 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 19fde3e2f1d..a2acdfddf25 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 00000000000..1c0a01d8ef6 --- /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 740c609adda..2d59e859300 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 d5c50b30e0c..992cfd92b02 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 1c0a01d8ef6..7f5cf9394e0 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); });