Update NextJS 15.1

[sakit0]

Summary

Update NextJS 15.1

Related Issue

#212

Changes

Testing

Other Information

[vercel]

@sakit0 is attempting to deploy a commit to the Edge Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
giselle	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 16, 2024 11:24am

[toyamarinyon]

I don't see any problem with turning Override off.

[sakit0]

Exactly!

[sakit0]

Fixed.
11a1a8e

[shige]

@sakit0 Could you please check this? It didn't work during the operation check, and an error log was displayed. 🙏

https://vercel.com/r06-edge/giselle/logs?selectedLogId=4qd7j-1734341227963-53adc6408a18

⨯ ReferenceError: agentId is not defined
    at S (.next/server/app/(playground)/p/[agentId]/canary/page.js:1:7611) {
  digest: '3057732266'
}

https://vercel.com/r06-edge/giselle/logs?selectedLogId=4qd7j-1734341228451-c1ebedbc3a73

⨯ Error: Flow with id flw_yv8pz65uc9e7gtqb98ru5kag not found
    at m (.next/server/app/(playground)/p/[agentId]/canary/page.js:99:93398)
    at async U (.next/server/app/(playground)/p/[agentId]/canary/page.js:1:8206) {
  digest: '2301578653'
}

[sakit0]

@shige
Previously, agentId was referenced directly within the server action, which caused issues after upgrading to Next.js 15.1. This commit updates the persistGraph server action to accept agentId as an explicit argument, ensuring that the necessary data is passed correctly from the client side and maintaining proper scoping for server-side operations.

49844e8

[shige]

LGTM! 🎉

[toyamarinyon]

@sakit0 Please let me know where I can find the change logs that are relevant to this issue?

Previously, agentId was referenced directly within the server action, which caused issues after upgrading to Next.js 15.1.

I know that defining a Server Action inside a component creates a closure where the action has access to the outer function's scope. And I understand that this behaviour has not changed in Next.js 15.1.

https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#closures-and-encryption

[sakit0]

@toyamarinyon
I'm also puzzled as to why it resulted in an error.
After reviewing the issues for Next 15.1, I suspect the two listed below may be relevant, but it's not definite.

vercel/next.js#73521
vercel/next.js#73471

[toyamarinyon]

I see, please give me a moment to try it locally.

[toyamarinyon]

I could reproduce the error on persistGraph on my local env, but closure works on the updateAgentName function.

It seems that there is a problem with the way GraphContextProvider is implemented, not with Next15.1.

I will investigate the cause a little further.

[toyamarinyon]

Note

Of course, it could be 49844e8, but I am inclined to find the cause and understand it, because I believe that it is a potentially bad implementation that does not allow the use of the features provided by the Framework.

[toyamarinyon]

@sakit0 I don't know why I was able to fix it, but this fixed it. I would like this to be used instead of 49844e8 if it is to be corrected in that environment.

diff --git a/app/(playground)/p/[agentId]/canary/page.tsx b/app/(playground)/p/[agentId]/canary/page.tsx
index 5220a1f7..1476f5f7 100644
--- a/app/(playground)/p/[agentId]/canary/page.tsx
+++ b/app/(playground)/p/[agentId]/canary/page.tsx
@@ -63,19 +63,18 @@ export default async function Page({
 	async function persistGraph(graph: Graph) {
 		"use server";
 		const { url } = await putGraph(graph);
+		const blobList = await list({
+			prefix: buildGraphFolderPath(graph.id),
+		});
+		const oldBlobUrls = blobList.blobs
+			.filter((b) => b.url !== url)
+			.map((b) => b.url);
 		await db
 			.update(agents)
 			.set({
 				graphUrl: url,
 			})
 			.where(eq(agents.id, agentId));
-		const blobList = await list({
-			prefix: buildGraphFolderPath(graph.id),
-		});
-
-		const oldBlobUrls = blobList.blobs
-			.filter((blob) => blob.url !== url)
-			.map((blob) => blob.url);
 		if (oldBlobUrls.length > 0) {
 			await del(oldBlobUrls);
 		}

[sakit0]

@toyamarinyon
Oh! That's great! I have no problem with this change. check in your own environment.
How did you arrive at this change?

[toyamarinyon]

@sakit0

First, I created a new Next.js application using npx create-next-app@latest and wrote the following page.tsx to verify if Closures were no longer working with Server Actions:

// page.tsx
export default function Home() {
  const msg = "hello";
  async function handleClick() {
    "use server";
    console.log(msg);
  }
  return (
    <button type="button" onClick={handleClick}>
      hello
    </button>
  );
}

The code worked without any issues. At this point, I realized this wasn't a widespread problem with Next 15.1, so I suspected the issue might be with how Closures are being used in Giselle.

Next, I checked if the same error occurred with the updateAgentName Server Action in the page that was throwing errors, as it references agentId through a Closure. This also worked fine.

This meant the issue had to be with either the persistGraph Server Action or the GraphContextProvider that receives Server Actions as props, so I proceeded to isolate the problem.

To narrow it down, I added an onPersistAction prop to GraphContextProvider and passed updateAgentName to it to see if it would trigger the error. I also added a test button for verification.
The code can be found here:
toyamarinyon@6f70430

Contrary to my expectations that it would fail and lead me to investigate the Provider code, it worked fine.

This indicated that the issue was with persistGraph. When I emptied the function and added code back line by line, the error occurred when adding this specific code:

const oldBlobUrls = blobList.blobs
			.filter((b) => b.url !== url)
			.map((b) => b.url);

When I changed the order of operations, it worked properly. I don't fully understand why this happens, so I plan to create an issue on Next.js once I can create a minimal reproduction of the problem.

[toyamarinyon]

I apologize for #247 where I changed the Playground from /p/:agentId/canary to /p/:agentId, which resulted in a large diff. You may need to recreate the Pull Request.

#247 is planned to be merged tomorrow morning.