Skip to content

[test] Add failing test for useActionState with 'use cache' #86292

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 19, 2025
+57 −3
Merged

[test] Add failing test for useActionState with 'use cache' #86292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions test/e2e/app-dir/use-cache/app/(partially-static)/use-action-state-separate-export/cached.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use cache'

const getRandomValue = async () => {
return Math.random()
}

export { getRandomValue }
15 changes: 15 additions & 0 deletions test/e2e/app-dir/use-cache/app/(partially-static)/use-action-state-separate-export/form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use client'

import { useActionState } from 'react'
import { getRandomValue } from './cached'

export function Form() {
const [result, formAction, isPending] = useActionState(getRandomValue, -1)

return (
<form action={formAction}>
<button id="submit-button">Submit</button>
<p>{isPending ? 'loading...' : result}</p>
</form>
)
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/use-cache/app/(partially-static)/use-action-state-separate-export/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Form } from './form'

export default function Page() {
return <Form />
}
4 changes: 1 addition & 3 deletions test/e2e/app-dir/use-cache/app/(partially-static)/use-action-state/cached.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use cache'

export async function getRandomValue() {
const v = Math.random()
console.log(v)
return v
return Math.random()
}
29 changes: 29 additions & 0 deletions test/e2e/app-dir/use-cache/use-cache.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ describe('use-cache', () => {
'/static-class-method',
withCacheComponents && '/unhandled-promise-regression',
'/use-action-state',
'/use-action-state-separate-export',
'/with-server-action',
].filter(Boolean)
)
Expand Down Expand Up @@ -709,6 +710,34 @@ describe('use-cache', () => {
})
})

// TODO: This test doesn't work currently because the compiler doesn't
// properly compute the server reference information byte that includes the
// function arity. Without this information, the client can't optimize the
// arguments it sends to the server, so the (unused) previous state is also
Comment on lines +714 to +716
Copy link
Member

@eps1lon eps1lon Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does that currently work? Do we just send all arguments if the Compiler sees arguments or rest parameters?

Copy link
Contributor Author

@unstubbable unstubbable Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is handled here:

next.js/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts

Lines 95 to 103 in e66e0ea

const info = extractInfoFromServerReferenceId(actionId)
// TODO: Currently, we're only omitting unused args for the experimental "use
// cache" functions. Once the server reference info byte feature is stable, we
// should apply this to server actions as well.
const usedArgs =
info.type === 'use-cache' ? omitUnusedArgs(actionArgs, info) : actionArgs
const body = await encodeReply(usedArgs, { temporaryReferences })

We're using the server reference information byte in the server reference ID to filter the arguments that we send to the server.

Copy link
Contributor Author

@unstubbable unstubbable Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we just send all arguments if the Compiler sees arguments or rest parameters?

If the compiler can't determine the arity we're sending all arguments unfiltered. So this opts out of the optimization. (Side note: Using arguments is forbidden in server functions, which is enforced by the compiler.)

Copy link
Member

@eps1lon eps1lon Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the thing I'm interested in. What's setting these bits?

Copy link
Contributor Author

@unstubbable unstubbable Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is done here:

next.js/crates/next-custom-transforms/src/transforms/server_actions.rs

Lines 279 to 339 in e66e0ea

// Prepend an extra byte to the ID, with the following format:
// 0 000000 0
// ^type ^arg mask ^rest args
//
// The type bit represents if the action is a cache function or not.
// For cache functions, the type bit is set to 1. Otherwise, it's 0.
//
// The arg mask bit is used to determine which arguments are used by
// the function itself, up to 6 arguments. The bit is set to 1 if the
// argument is used, or being spread or destructured (so it can be
// indirectly or partially used). The bit is set to 0 otherwise.
//
// The rest args bit is used to determine if there's a ...rest argument
// in the function signature. If there is, the bit is set to 1.
//
// For example:
//
// async function foo(a, foo, b, bar, ...baz) {
// 'use cache';
// return a + b;
// }
//
// will have it encoded as [1][101011][1]. The first bit is set to 1
// because it's a cache function. The second part has 1010 because the
// only arguments used are `a` and `b`. The subsequent 11 bits are set
// to 1 because there's a ...rest argument starting from the 5th. The
// last bit is set to 1 as well for the same reason.
let type_bit = if is_cache { 1u8 } else { 0u8 };
let mut arg_mask = 0u8;
let mut rest_args = 0u8;
if let Some(params) = params {
// TODO: For the current implementation, we don't track if an
// argument ident is actually referenced in the function body.
// Instead, we go with the easy route and assume defined ones are
// used. This can be improved in the future.
for (i, param) in params.iter().enumerate() {
if let Pat::Rest(_) = param.pat {
// If there's a ...rest argument, we set the rest args bit
// to 1 and set the arg mask to 0b111111.
arg_mask = 0b111111;
rest_args = 0b1;
break;
}
if i < 6 {
arg_mask |= 0b1 << (5 - i);
} else {
// More than 6 arguments, we set the rest args bit to 1.
// This is rare for a Server Action, usually.
rest_args = 0b1;
break;
}
}
} else {
// If we can't determine the arguments (e.g. not statically analyzable),
// we assume all arguments are used.
arg_mask = 0b111111;
rest_args = 0b1;
}
result.push((type_bit << 7) | (arg_mask << 1) | rest_args);

// sent as an argument, leading to cache misses.
it.failing(
'works with useActionState if previousState parameter is not used in "use cache" function (separate export)',
async () => {
const browser = await next.browser('/use-action-state-separate-export')

let value = await browser.elementByCss('p').text()
expect(value).toBe('-1')

await browser.elementByCss('button').click()

await retry(async () => {
value = await browser.elementByCss('p').text()
expect(value).toMatch(/\d\.\d+/)
})

await browser.elementByCss('button').click()

await retry(async () => {
expect(await browser.elementByCss('p').text()).toBe(value)
})
Comment on lines +733 to +737
Copy link
Member

@eps1lon eps1lon Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this potentially flaky in case we assert on the content before the form actually goes into pending?

I'd instead log in an Effect that we're pending and when the new value comes in. And then assert on those logs. Kinda like we do in our React tests with Scheduler.log.

Copy link
Contributor Author

@unstubbable unstubbable Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's needed here. See #72506 (comment)

In Datadog I could only find failures (for the existing test with the same setup) that seem to be triggered by other failed tests abandoning the job while this test suite was still running, if I understand this correctly. I actually didn't know this can happen, and it kinda skews our flakiness metrics!

}
)

it('works with "use cache" in method props', async () => {
const browser = await next.browser('/method-props')

Expand Down
Loading