Commit dependency order changes to graph (#1173)

Summary:
Pull Request resolved: #1173

Changelog:
* **[Fix]**: Edge case leading to invalid bundles when only the *order* of dependencies within a module has changed.

This is a fix for a bug introduced in D52457312 - confirmed by backporting the new unit test and observing that it passes on D52457312's base commit.

Reviewed By: hoxyq

Differential Revision: D52660716

fbshipit-source-id: 1d4aac4c30d7726af528e31f4dc50a8f62688e14

Author: motiz88

packages/metro/src/DeltaBundler/Graph.js

@@ -421,7 +421,7 @@ export class Graph<T = MixedOutput> {
       }
     }
 
-    // Diff dependencies (1/2): remove dependencies that have changed or been removed.
+    // Diff dependencies (1/3): remove dependencies that have changed or been removed.
     let dependenciesRemoved = false;
     for (const [key, prevDependency] of previousDependencies) {
       const curDependency = currentDependencies.get(key);
@@ -434,7 +434,7 @@ export class Graph<T = MixedOutput> {
       }
     }
 
-    // Diff dependencies (2/2): add dependencies that have changed or been added.
+    // Diff dependencies (2/3): add dependencies that have changed or been added.
     let dependenciesAdded = false;
     if (!commitOptions.onlyRemove) {
       for (const [key, curDependency] of currentDependencies) {
@@ -456,11 +456,21 @@ export class Graph<T = MixedOutput> {
       }
     }
 
+    // Diff dependencies (3/3): detect changes in the ordering of dependency
+    // keys, which must be committed even if no other changes were made.
+    const previousDependencyKeys = [...previousDependencies.keys()];
+    const dependencyKeysChangedOrReordered =
+      currentDependencies.size !== previousDependencies.size ||
+      [...currentDependencies.keys()].some(
+        (currentKey, index) => currentKey !== previousDependencyKeys[index],
+      );
+
     if (
       previousModule != null &&
       !transformOutputMayDiffer(previousModule, nextModule) &&
       !dependenciesRemoved &&
-      !dependenciesAdded
+      !dependenciesAdded &&
+      !dependencyKeysChangedOrReordered
     ) {
       // We have not operated on nextModule, so restore previousModule
       // to aid diffing. Don't add this path to delta.touched.

packages/metro/src/DeltaBundler/__tests__/Graph-test.js

@@ -3913,3 +3913,32 @@ describe('recovery from transform and resolution errors', () => {
     });
   });
 });
+
+test('when only the order of transformer dependencies changes, the resolved dependencies should be reordered too', async () => {
+  await graph.initialTraverseDependencies(options);
+  expect([
+    ...nullthrows(graph.dependencies.get('/foo')).dependencies.values(),
+  ]).toEqual([
+    objectContaining({absolutePath: '/bar'}),
+    objectContaining({absolutePath: '/baz'}),
+  ]);
+
+  Actions.modifyFile('/foo');
+  const transformerDeps = nullthrows(mockedDependencyTree.get('/foo'));
+  mockedDependencyTree.set('/foo', [...transformerDeps].reverse());
+
+  expect(
+    getPaths(await graph.traverseDependencies([...files], options)),
+  ).toEqual({
+    added: new Set(),
+    modified: new Set(['/foo']),
+    deleted: new Set(),
+  });
+
+  expect([
+    ...nullthrows(graph.dependencies.get('/foo')).dependencies.values(),
+  ]).toEqual([
+    objectContaining({absolutePath: '/baz'}),
+    objectContaining({absolutePath: '/bar'}),
+  ]);
+});