clerk / javascript

Official JavaScript repository for Clerk authentication
https://clerk.com
MIT License
1.19k stars 271 forks source link

ClerkProvider + auth + everything not compatible with next PPR (next 14) #2501

Closed juliuslipp closed 10 months ago

juliuslipp commented 11 months ago

Preliminary Checks

Reproduction / Replay Link

https://github.com/juliuslipp/clerk

Publishable key

pk_test_ZmlybS1waWdsZXQtNjQuY2xlcmsuYWNjb3VudHMuZGV2JA

Description

Steps to reproduce:

  1. Step 1 bun install
  2. Step 2 bun run build

see error

remove clerk provider and uncomment headers/cookies in app/recoomended-products.ts

bun run build

see no error

Expected behavior: no error

Actual behavior: error

Environment

System:
    OS: macOS 14.2.1
    CPU: (10) arm64 Apple M2 Pro
    Memory: 125.20 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.0 - ~/.nvm/versions/node/v18.18.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.1.0 - ~/.bun/bin/npm
    pnpm: 8.6.9 - ~/Library/pnpm/pnpm
    bun: 1.0.21 - ~/.bun/bin/bun
  Browsers:
    Chrome: 120.0.6099.199
    Safari: 17.2.1
  npmPackages:
    @clerk/nextjs: ^4.29.1 => 4.29.1
    @heroicons/react: 2.1.1 => 2.1.1
    @openstatus/react: ^0.0.3 => 0.0.3
    @tailwindcss/forms: 0.5.7 => 0.5.7
    @tailwindcss/typography: 0.5.10 => 0.5.10
    @types/node: 20.10.7 => 20.10.7
    @types/react: 18.2.47 => 18.2.47
    @types/react-dom: 18.2.18 => 18.2.18
    @vercel/git-hooks: 1.0.0 => 1.0.0
    autoprefixer: 10.4.16 => 10.4.16
    clsx: 2.1.0 => 2.1.0
    date-fns: 3.1.0 => 3.1.0
    dinero.js: 2.0.0-alpha.8 => 2.0.0-alpha.8
    eslint: 8.56.0 => 8.56.0
    eslint-config-next: 14.0.4 => 14.0.4
    geist: 1.2.0 => 1.2.0
    lint-staged: 15.2.0 => 15.2.0
    next: 14.0.5-canary.42 => 14.0.5-canary.42
    postcss: 8.4.33 => 8.4.33
    prettier: 3.1.1 => 3.1.1
    prettier-plugin-tailwindcss: 0.5.11 => 0.5.11
    react: 18.2.0 => 18.2.0
    react-dom: 18.2.0 => 18.2.0
    tailwindcss: 3.4.1 => 3.4.1
    typescript: 5.3.3 => 5.3.3
LekoArts commented 11 months ago

Hi, thanks for the issue!

The Partial Prerendering feature is marked as highly experimental:

Warning: Partial Prerendering is an experimental feature and is currently not suitable for production environments.

It also has these gotchas:

Partial Prerendering does not yet apply to client-side navigations. We are actively working on this.

There's a good chance that things are not working because something isn't implemented/right yet in Next.js itself. It's also possible that we'll need to update something on our end. Until this feature is in beta or soon to be released we'll however not spend time on this as things aren't settled just yet.

I'd encourage you to open an issue on Next.js and see what the root cause of the error might be. Thanks!

juliuslipp commented 11 months ago

Hey @LekoArts,

Thanks for looking into this. While I see, that this feature is still experimental, I don't agree with your statement that this lies on nexts side.

React throws an error when server features are used (headers, cookies) that is used to determine if a component needs to opt out of static rendering (and will be partially prerendered). Since your implementation uses headers() within a try-catch blog:

Prerendering /_not-found needs to partially bail out because something dynamic was used. React throws a special object to indicate where we need to bail out but it was caught by a try/catch or a Promise was not awaited. These special objects should not be caught by your own try/catch. Learn more: https://nextjs.org/docs/messages/ppr-caught-error
 ⚠ The following error was thrown during build, and may help identify the source of the issue:
 ⨯ Error: Clerk: auth() and currentUser() are only supported in App Router (/app directory).
If you're using /pages, try getAuth() instead.
Original error: Error: This page needs to bail out of prerendering at this point because it used headers. React throws this special object to indicate where. It should not be caught by your own try/catch. Learn more: https://nextjs.org/docs/messages/ppr-caught-error

I see that this feature is still experimental, but I am certain it will make it to prod. and also certain that the implementation will be similar to what it is right now, since they don't want to introduce new apis for this.

Here is the matching lines of code: https://github.com/clerk/javascript/blob/main/packages/nextjs/src/app-router/server/utils.ts#L4C1-L21C3

A simple solution would be to determine and integrate the new error message into the rethrow block of buildRequestLike and simply rethrow this error as well.

It is indeed annoying, since I would love to use PPR right now and clerk is the only thing holding me back.

Should be a simple fix. Please reopen. Happy to help :)

LekoArts commented 10 months ago

Okay, if you think that change alone would make it work, then please put up a PR. As I said we have other priorities right now but happy to look at a PR.

You can (quickly) test this yourself locally:

  1. Remove your .next folder
  2. Go into node_modules/@clerk/nextjs/dist/esm/server/utils.js and make your change there
  3. Start the server
juliuslipp commented 10 months ago

hey @LekoArts,

Just created a PR. Tested it and fixes the issue :) #2518

Do you roughly know when I can expect it to work with the npm package?