Turbopack: deterministic server action order (#76905)

`HashMap` strikes again, causing the server action manifest order to be non-deterministic.

Less test flakeyness for `app-dir - server-action-period-hash-custom-key should have different manifest if the encryption key from process env is same`

Author: mischnic

crates/next-api/src/module_graph.rs

@@ -213,7 +213,7 @@ impl ServerActionsGraph {
                     return Ok(Vc::cell(Default::default()));
                 }
 
-                let mut result = FxHashMap::default();
+                let mut result = FxIndexMap::default();
                 graph.traverse_from_entry(entry, |node| {
                     if let Some(node_data) = data.get(&node.module) {
                         result.insert(node.module, *node_data);

crates/next-api/src/server_actions.rs

@@ -7,7 +7,6 @@ use next_core::{
     },
     util::NextRuntime,
 };
-use rustc_hash::FxHashMap;
 use swc_core::{
     atoms::Atom,
     common::comments::Comments,
@@ -406,7 +405,7 @@ struct OptionActionMap(Option<ResolvedVc<ActionMap>>);
 type LayerAndActions = (ActionLayer, ResolvedVc<ActionMap>);
 /// A mapping of every module module containing Server Actions, mapping to its layer and actions.
 #[turbo_tasks::value(transparent)]
-pub struct AllModuleActions(FxHashMap<ResolvedVc<Box<dyn Module>>, LayerAndActions>);
+pub struct AllModuleActions(FxIndexMap<ResolvedVc<Box<dyn Module>>, LayerAndActions>);
 
 #[turbo_tasks::function]
 pub async fn map_server_actions(graph: Vc<SingleModuleGraph>) -> Result<Vc<AllModuleActions>> {

test/production/app-dir/server-action-period-hash/server-action-period-hash-custom-key.test.ts

@@ -1,24 +1,10 @@
-import { nextTestSetup } from 'e2e-utils'
+import { nextTestSetup, type NextInstance } from 'e2e-utils'
 
-async function getServerActionManifest(next) {
-  const content = await next.readFile(
+async function getServerActionManifestNodeKeys(next: NextInstance) {
+  const manifest = await next.readJSON(
     '.next/server/server-reference-manifest.json'
   )
-  return JSON.parse(content)
-}
-
-function compareServerActionManifestKeys(a, b, equal) {
-  a = a.node
-  b = b.node
-
-  const keysA = Object.keys(a)
-  const keysB = Object.keys(b)
-
-  if (equal) {
-    expect(keysA).toEqual(keysB)
-  } else {
-    expect(keysA).not.toEqual(keysB)
-  }
+  return Object.keys(manifest.node)
 }
 
 describe('app-dir - server-action-period-hash-custom-key', () => {
@@ -27,32 +13,30 @@ describe('app-dir - server-action-period-hash-custom-key', () => {
     skipStart: true,
   })
 
-  it('should have different manifest if the encryption key from process env is changed', async () => {
+  it('should have a different manifest if the encryption key from process env is changed', async () => {
     process.env.NEXT_SERVER_ACTIONS_ENCRYPTION_KEY = 'my-secret-key1'
     await next.build()
     delete process.env.NEXT_SERVER_ACTIONS_ENCRYPTION_KEY
-    const firstManifest = await getServerActionManifest(next)
+    const firstActionIds = await getServerActionManifestNodeKeys(next)
 
     process.env.NEXT_SERVER_ACTIONS_ENCRYPTION_KEY = 'my-secret-key2'
     await next.build()
     delete process.env.NEXT_SERVER_ACTIONS_ENCRYPTION_KEY
+    const secondActionIds = await getServerActionManifestNodeKeys(next)
 
-    const secondManifest = await getServerActionManifest(next)
-
-    compareServerActionManifestKeys(firstManifest, secondManifest, false)
+    expect(firstActionIds).not.toEqual(secondActionIds)
   })
 
-  it('should have different manifest if the encryption key from process env is same', async () => {
+  it('should have the same manifest if the encryption key from process env is same', async () => {
     process.env.NEXT_SERVER_ACTIONS_ENCRYPTION_KEY = 'my-secret-key'
     await next.build()
-    const firstManifest = await getServerActionManifest(next)
+    const firstActionIds = await getServerActionManifestNodeKeys(next)
 
     await next.remove('.next') // dismiss cache
     await next.build() // build with the same secret key
     delete process.env.NEXT_SERVER_ACTIONS_ENCRYPTION_KEY
+    const secondActionIds = await getServerActionManifestNodeKeys(next)
 
-    const secondManifest = await getServerActionManifest(next)
-
-    compareServerActionManifestKeys(firstManifest, secondManifest, true)
+    expect(firstActionIds).toEqual(secondActionIds)
   })
 })

test/production/app-dir/server-action-period-hash/server-action-period-hash.test.ts

@@ -1,24 +1,10 @@
-import { nextTestSetup } from 'e2e-utils'
+import { nextTestSetup, type NextInstance } from 'e2e-utils'
 
-async function getServerActionManifest(next) {
-  const content = await next.readFile(
+async function getServerActionManifestNodeKeys(next: NextInstance) {
+  const manifest = await next.readJSON(
     '.next/server/server-reference-manifest.json'
   )
-  return JSON.parse(content)
-}
-
-function compareServerActionManifestKeys(a, b, equal) {
-  a = a.node
-  b = b.node
-
-  const keysA = Object.keys(a)
-  const keysB = Object.keys(b)
-
-  if (equal) {
-    expect(keysA).toEqual(keysB)
-  } else {
-    expect(keysA).not.toEqual(keysB)
-  }
+  return Object.keys(manifest.node)
 }
 
 describe('app-dir - server-action-period-hash', () => {
@@ -29,23 +15,23 @@ describe('app-dir - server-action-period-hash', () => {
 
   it('should have same manifest between continuous two builds', async () => {
     await next.build()
-    const firstManifest = await getServerActionManifest(next)
+    const firstActionIds = await getServerActionManifestNodeKeys(next)
 
     await next.build()
-    const secondManifest = await getServerActionManifest(next)
+    const secondActionIds = await getServerActionManifestNodeKeys(next)
 
-    compareServerActionManifestKeys(firstManifest, secondManifest, true)
+    expect(firstActionIds).toEqual(secondActionIds)
   })
 
   it('should have different manifest between two builds with period hash', async () => {
     await next.build()
-    const firstManifest = await getServerActionManifest(next)
+    const firstActionIds = await getServerActionManifestNodeKeys(next)
 
     await next.remove('.next') // dismiss cache
     await next.build()
 
-    const secondManifest = await getServerActionManifest(next)
+    const secondActionIds = await getServerActionManifestNodeKeys(next)
 
-    compareServerActionManifestKeys(firstManifest, secondManifest, false)
+    expect(firstActionIds).not.toEqual(secondActionIds)
   })
 })