Skip to content
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

Next js 15 with dynamicIO: true returned server error #8277

Closed
yudistiraashadi opened this issue Nov 11, 2024 · 11 comments · May be fixed by #8315
Closed

Next js 15 with dynamicIO: true returned server error #8277

yudistiraashadi opened this issue Nov 11, 2024 · 11 comments · May be fixed by #8315

Comments

@yudistiraashadi
Copy link

Describe the bug

Running the "React Example: Nextjs App Prefetching" from the docs with dynamicIO: true setting on next.config.ts return this error shown below. The code is available here.

Image

Your minimal, reproducible example

https://github.com/yudistiraashadi/next15-tanstack-query-canary

Steps to reproduce

  1. Install next.js 15 canary npx create-next-app@canary
  2. Install tanstack query and devtools pnpm add @tanstack/react-query and pnpm add -D @tanstack/react-query-devtools
  3. Copy all the code from the "React Example: Nextjs App Prefetching" here, to replace all the code inside src folders on the nextjs app folder (or /src folder, I'm using src folder btw)
  4. Run dev server pnpm run dev and open localhost:3000

Expected behavior

No error

How often does this bug happen?

Every time

Screenshots or Videos

Screencast.from.11-11-24.15.53.38.webm

Platform

  • OS: Ubuntu 22.04
  • Browser: Chrome
  • Tanstack Query version: 5.59.20
  • Next.js 15 canary: 15.0.4-canary.5

Tanstack Query adapter

None

TanStack Query version

5.59.20

TypeScript version

5.6.3

Additional context

No response

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 11, 2024

Not sure if I discussed this with you, but it came up on Discord as well. The reason is likely that we initiate our mutationIds with Date.now:

this.#mutationId = Date.now()

It doesn’t have to be Date.now, but it needs to be a unique id that we can increment and that doesn’t overlap with ids when they become restored them from localStorage.

If you have an idea what that could be, please file a PR.

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 11, 2024

Actually, we only serialize the scope, which defaults to String(mutation.mutationId):

function scopeFor(mutation: Mutation<any, any, any, any>) {
return mutation.options.scope?.id ?? String(mutation.mutationId)
}

So we can also initialize mutations with 0 (like it was before we had that feature) IF we find a way to generate a unique string as a scope for mutations. We sadly can’t use crypto.randomUUID because it’s not available in all the browsers we support.

@yudistiraashadi
Copy link
Author

yudistiraashadi commented Nov 11, 2024

(edit) DISCLAIMER: THIS DOESN'T WORK !!

Honestly, I can't think of any way to consistently generate unique strings for older browsers. Currently, I tried to use await connection() based on the Next.js docs here. Basically creating 2 getQueryClient(), one for server and one for browser. I'm not sure what the await connection() does or the performance implication of awaiting before new QueryClient(queryClientConfig) but at least it doesn't throw any error now.

import {
  QueryClient,
  defaultShouldDehydrateQuery,
  type QueryClientConfig,
} from "@tanstack/react-query";
import { connection } from "next/server";

const queryClientConfig: QueryClientConfig = {
  defaultOptions: {
    queries: {
      staleTime: 60 * 1000,
    },
    dehydrate: {
      // include pending queries in dehydration
      shouldDehydrateQuery: (query) =>
        defaultShouldDehydrateQuery(query) || query.state.status === "pending",
    },
  },
};

let browserQueryClient: QueryClient | undefined = undefined;

export async function getServerQueryClient() {
  await connection();
  return new QueryClient(queryClientConfig);
}

export function getBrowserQueryClient() {
  // Browser: make a new query client if we don't already have one
  // This is very important, so we don't re-make a new client if React
  // suspends during the initial render. This may not be needed if we
  // have a suspense boundary BELOW the creation of the query client
  if (!browserQueryClient) {
    browserQueryClient = new QueryClient(queryClientConfig);
  }

  return browserQueryClient;
}

@yudistiraashadi
Copy link
Author

I'm writing this as a note for future developers. I ended up patching @tanstack/query-core to use crypto.randomUUID() according to @TkDodo comments #8277 (comment) and #8277 (comment). If anyone has a better solution please comment here.

@firatciftci
Copy link

Apologies for commenting on a closed issue, but do we have a final (and official) verdict on what to do to be able to use TanStack Query with Next.js 15 Canary? I understand that it is an experimental build and so the package maintainers should not spend their time and effort to support something that is not finalized and stable, but if there is any way to utilize TanStack Query with the dynamicIO and PPR features and test it to be ready for an upgrade once they land in main, I would love to learn how.

@ali-idrizi
Copy link

crypto.randomUUID() also throws the same error according to https://nextjs.org/docs/messages/next-prerender-crypto.

Perhaps performance.timeOrigin + performance.now() is a good option, just like the error suggests? It's generally equal to Date.now() and it seems to have pretty good browser support.

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 20, 2024

Didn’t know about performance.timeOrigin. The suggestion to use performance.timeOrigin + performance.now() seems good 👍 . Would you want to create a PR for this @ali-idrizi ?

@ali-idrizi
Copy link

Absolutely! I just created the PR that fixes this.

@jk2908
Copy link

jk2908 commented Nov 26, 2024

Before the patch is released, adding a Suspense boundary w/ null fallback directly above your Providers works.

@spoldman
Copy link

spoldman commented Dec 11, 2024

I implemented a temporary fix for this issue until ali-idrizi PR is merged using the code below

import { MutationCache } from '@tanstack/react-query';

export class MutationCache_TEMP_FIX extends MutationCache {
  constructor() {
    const old = Date.now;
    Date.now = () => Math.round(performance.timeOrigin + performance.now());
    super();
    Date.now = old;
  }
}

Then, I applied the new implementation as follows:

return new QueryClient({
    mutationCache: new MutationCache_TEMP_FIX(),
    defaultOptions: {
     ...
    },
  });

I know this solution isn't ideal, but it works as a stopgap measure while waiting for the official fix.

Disclaimer: I haven’t tested this solution thoroughly, but it did get my build working for now.

@gnoff
Copy link

gnoff commented Dec 18, 2024

Hey folks, I implemented dynamicIO in Next.js. Just chiming in to say that you can use Next.js 15 without turning this flag on yet. We still have changes to make that are likely to be breaking for you if you use this highly experimental mode. We will ship a beta early 2025 hopefully as part of 15.2 (no promises) where we will actually encourage use.

The fix to make this use timeOrigin might work today but because it is so tempting to use this value for non-telemetry purposes we may end up having to treat it similar to Date.now() in that it must be excluded from prerenders unless explicitly cached so I don't think this approach is going to solve the compatability issue.

You've already discovered you can do things like await connection() prior to rendering the mutationCache which fixes the issue insofar as it excludes the mutationCache from prerendering. But that's effectively turning off prerendering which is likely not what you want.

I put together a PR to potentially address the compatibility within MutationCache itself here: #8450

I'm not familiar with all the constraints that might be in play so it's possible my proposal isn't suitable for some reason but at the moment I think it is, so when the project maintainers have a chance to review we can hopefully land this and unblock this usage with the dynamicIO flag. Thanks for your patience :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants