feat(expo): update expo sync-deps executor (#26086)

## Current Behavior When running `@nx/expo:sync-deps` it includes many unexpected dependencies. If you add a backend project as an implicit dependency to the expo project, then all of the backend project's dependencies are included in the expo package.json when running `@nx/expo:sync-deps` You can use the `exclude` option, but with hundreds of excluded packages, this adds a lot of bloat to the targets in `project.json` ## Expected Behavior Ideally, when using `sync-deps` as a part of the `build`, only packages necessary in the context of the `build` would be synced. Since the packages from `implicitDependencies` aren't typically relevant to the expo build, we should optionally be able to not include them. ## Notes ### Default Value I made the default `excludeImplicit: false` so that it doesn't diverge from current behavior/expectations — but it's possible that it would make more sense to have it be `true` by default — would defer to y'all on that question. ## Additional Considerations ### Other Possible Options > [!NOTE] > Let me know if you're interested in PRs to add any of these <details> <summary>Other Possible Options</summary> Here are some other options which might be worth considering. - [x] `excludeImplicit` <- _added in this PR_ - [ ] `onlyNativeDependencies`* - [ ] `onlyPodInstallDependencies`* - [ ] `traceDependencyPaths`** - [ ] `excludeDevDependencies`*** - [ ] `matchRootPackageJsonCatgeory`*** - [ ] `onlySrcFiles`**** - [ ] `filterByCacheInputs`**** #### Only Native / Pod Installs* Based on the discussion in issue #18788 it seem like the primary reason for `sync-deps`, is to support pod install. #### Trace Dependency Paths** When I was originally debugging "why is axios being added?" — before I'd realized about the `implicitDependencies` — I wrote a utility to output the trace for the included packages — that's how I realized what was going on. Could be a useful feature addition. ![image](https://github.com/nrwl/nx/assets/2213636/e1cb1511-c518-47d8-85fb-69c6a6d88058) #### Deps vs DevDeps*** By default, the `sync-deps` feature will find all dependencies including(eg jest, storybook) and add them to `package.json` under the `"dependencies":` key. It might be useful to either match the root `package.json`'s categorization or just exclude devDependencies altogether. #### File aware filtering**** Currently the `findAllNpmDependencies` is filtering some hardcoded external nodes: ``` 'npm:@nx/react-native', 'npm:@nrwl/react-native', 'npm:@nx/expo', 'npm:@nrwl/expo', ``` These are in the dependency graph because they are used as executors in `project.json` targets. It might be useful to derive these exclusions dynamically, by only considering relevant productions files. A simple approach would be to only consider dependencies that stem from files in the `src` directory A more robust alternative would be to read the cache inputs from the calling target, and filter dependencies based on matching files </details> ### Fingerprinting? <details> <summary>Fingerprinting</summary> There's a related matter having to do with `@expo/fingerprint` where having the native dependencies visible from the project-level `package.json` is important to getting accurate project-level fingerprints. The more ideal solution would be to use the Nx graph to handle the "fingerprinting" hash generation, but it would require some thought / feature design. So in the meantime the `sync-deps` (only need native deps) + `@expo/fingerprint` recourse seems like the best option. </details> Thanks!
nrwl · Jun 5, 2024 · 260562e · 260562e
1 parent 5a06daa
commit 260562e
Show file tree

Hide file tree

Showing 6 changed files with 213 additions and 119 deletions.
diff --git a/docs/generated/packages/expo/executors/sync-deps.json b/docs/generated/packages/expo/executors/sync-deps.json
@@ -27,6 +27,11 @@
         "type": "boolean",
         "description": "Copy all dependencies and devDependencies from the workspace root package.json.",
         "default": false
+      },
+      "excludeImplicit": {
+        "type": "boolean",
+        "description": "This will ignore npm packages from projects listed in implicitDependencies (e.g. backend API projects)",
+        "default": false
       }
     },
     "presets": []

diff --git a/packages/expo/src/executors/sync-deps/schema.d.ts b/packages/expo/src/executors/sync-deps/schema.d.ts
@@ -2,4 +2,5 @@ export interface ExpoSyncDepsOptions {
   include: string[] | string; // default is an empty array []
   exclude: string[] | string; // default is an empty array []
   all: boolean; // default is false
+  excludeImplicit: boolean; // default is false
 }
diff --git a/packages/expo/src/executors/sync-deps/schema.json b/packages/expo/src/executors/sync-deps/schema.json
@@ -28,6 +28,11 @@
       "type": "boolean",
       "description": "Copy all dependencies and devDependencies from the workspace root package.json.",
       "default": false
+    },
+    "excludeImplicit": {
+      "type": "boolean",
+      "description": "This will ignore npm packages from projects listed in implicitDependencies (e.g. backend API projects)",
+      "default": false
     }
   }
 }
diff --git a/packages/expo/src/executors/sync-deps/sync-deps.impl.ts b/packages/expo/src/executors/sync-deps/sync-deps.impl.ts
@@ -47,7 +47,8 @@ export default async function* syncDepsExecutor(
       typeof options.exclude === 'string'
         ? options.exclude.split(',')
         : options.exclude,
-      options.all
+      options.all,
+      options.excludeImplicit
     )
   );
 
@@ -62,11 +63,12 @@ export async function syncDeps(
   projectGraph: ProjectGraph = readCachedProjectGraph(),
   include: string[] = [],
   exclude: string[] = [],
-  all: boolean = false
+  all: boolean = false,
+  excludeImplicit: boolean = false
 ): Promise<string[]> {
   let npmDeps = all
     ? Object.keys(workspacePackageJson.dependencies || {})
-    : findAllNpmDependencies(projectGraph, projectName);
+    : findAllNpmDependencies(projectGraph, projectName, { excludeImplicit });
   let npmDevdeps = all
     ? Object.keys(workspacePackageJson.devDependencies || {})
     : [];

diff --git a/packages/expo/src/utils/find-all-npm-dependencies.spec.ts b/packages/expo/src/utils/find-all-npm-dependencies.spec.ts
@@ -1,103 +1,160 @@
 import { findAllNpmDependencies } from './find-all-npm-dependencies';
 import { DependencyType, ProjectGraph } from '@nx/devkit';
 
-test('findAllNpmDependencies', () => {
-  const graph: ProjectGraph = {
-    nodes: {
-      myapp: {
-        type: 'app',
-        name: 'myapp',
-        data: { files: [] },
+const graphFixture: ProjectGraph = {
+  nodes: {
+    myapp: {
+      type: 'app',
+      name: 'myapp',
+      data: { files: [], implicitDependencies: ['lib4'] },
+    },
+    lib1: {
+      type: 'lib',
+      name: 'lib1',
+      data: { files: [] },
+    },
+    lib2: {
+      type: 'lib',
+      name: 'lib2',
+      data: { files: [] },
+    },
+    lib3: {
+      type: 'lib',
+      name: 'lib3',
+      data: { files: [] },
+    },
+    lib4: {
+      type: 'lib',
+      name: 'lib4',
+      data: { files: [] },
+    },
+  } as any,
+  externalNodes: {
+    'npm:react-native-image-picker': {
+      type: 'npm',
+      name: 'npm:react-native-image-picker',
+      data: {
+        version: '1',
+        packageName: 'react-native-image-picker',
+      },
+    },
+    'npm:react-native-dialog': {
+      type: 'npm',
+      name: 'npm:react-native-dialog',
+      data: {
+        version: '1',
+        packageName: 'react-native-dialog',
+      },
+    },
+    'npm:react-native-snackbar': {
+      type: 'npm',
+      name: 'npm:react-native-snackbar',
+      data: {
+        version: '1',
+        packageName: 'react-native-snackbar',
       },
-      lib1: {
-        type: 'lib',
-        name: 'lib1',
-        data: { files: [] },
+    },
+    'npm:@nx/react-native': {
+      type: 'npm',
+      name: 'npm:@nx/react-native',
+      data: {
+        version: '1',
+        packageName: '@nx/react-native',
       },
-      lib2: {
-        type: 'lib',
-        name: 'lib2',
-        data: { files: [] },
+    },
+    'npm:axios': {
+      type: 'npm',
+      name: 'npm:axios',
+      data: {
+        version: '1',
+        packageName: 'axios',
       },
-      lib3: {
-        type: 'lib',
-        name: 'lib3',
-        data: { files: [] },
+    },
+  },
+  dependencies: {
+    myapp: [
+      { type: DependencyType.static, source: 'myapp', target: 'lib1' },
+      { type: DependencyType.static, source: 'myapp', target: 'lib2' },
+      { type: DependencyType.implicit, source: 'myapp', target: 'lib4' },
+      {
+        type: DependencyType.static,
+        source: 'myapp',
+        target: 'npm:react-native-image-picker',
       },
-    } as any,
-    externalNodes: {
-      'npm:react-native-image-picker': {
-        type: 'npm',
-        name: 'npm:react-native-image-picker',
-        data: {
-          version: '1',
-          packageName: 'react-native-image-picker',
-        },
+      {
+        type: DependencyType.static,
+        source: 'myapp',
+        target: 'npm:@nx/react-native',
       },
-      'npm:react-native-dialog': {
-        type: 'npm',
-        name: 'npm:react-native-dialog',
-        data: {
-          version: '1',
-          packageName: 'react-native-dialog',
-        },
+    ],
+    lib1: [
+      { type: DependencyType.static, source: 'lib1', target: 'lib2' },
+      {
+        type: DependencyType.static,
+        source: 'lib3',
+        target: 'npm:react-native-snackbar',
       },
-      'npm:react-native-snackbar': {
-        type: 'npm',
-        name: 'npm:react-native-snackbar',
-        data: {
-          version: '1',
-          packageName: 'react-native-snackbar',
-        },
+    ],
+    lib2: [{ type: DependencyType.static, source: 'lib2', target: 'lib3' }],
+    lib3: [
+      {
+        type: DependencyType.static,
+        source: 'lib3',
+        target: 'npm:react-native-dialog',
       },
-      'npm:@nx/react-native': {
-        type: 'npm',
-        name: 'npm:@nx/react-native',
-        data: {
-          version: '1',
-          packageName: '@nx/react-native',
-        },
+    ],
+    lib4: [
+      {
+        type: DependencyType.static,
+        source: 'lib4',
+        target: 'npm:axios',
       },
-    },
-    dependencies: {
-      myapp: [
-        { type: DependencyType.static, source: 'myapp', target: 'lib1' },
-        { type: DependencyType.static, source: 'myapp', target: 'lib2' },
-        {
-          type: DependencyType.static,
-          source: 'myapp',
-          target: 'npm:react-native-image-picker',
-        },
-        {
-          type: DependencyType.static,
-          source: 'myapp',
-          target: 'npm:@nx/react-native',
-        },
-      ],
-      lib1: [
-        { type: DependencyType.static, source: 'lib1', target: 'lib2' },
-        {
-          type: DependencyType.static,
-          source: 'lib3',
-          target: 'npm:react-native-snackbar',
-        },
-      ],
-      lib2: [{ type: DependencyType.static, source: 'lib2', target: 'lib3' }],
-      lib3: [
-        {
-          type: DependencyType.static,
-          source: 'lib3',
-          target: 'npm:react-native-dialog',
-        },
-      ],
-    },
-  };
+    ],
+  },
+};
+
+describe('findAllNpmDependencies', () => {
+  it('should return all npm dependencies of a project', () => {
+    const result = findAllNpmDependencies(graphFixture, 'myapp');
+
+    expect(result).toEqual([
+      'react-native-dialog',
+      'react-native-snackbar',
+      'axios',
+      'react-native-image-picker',
+    ]);
+  });
+
+  describe('when passed excludeImplicit option', () => {
+    it('should exclude implicit dependencies when `excludeImplicit` flag is true', () => {
+      const result = findAllNpmDependencies(
+        graphFixture,
+        'myapp',
+        { excludeImplicit: true },
+        new Set()
+      );
+
+      expect(result).toEqual([
+        'react-native-dialog',
+        'react-native-snackbar',
+        'react-native-image-picker',
+      ]);
+    });
 
-  const result = findAllNpmDependencies(graph, 'myapp');
+    it('should include implicit dependencies when `excludeImplicit` flag is false', () => {
+      const result = findAllNpmDependencies(
+        graphFixture,
+        'myapp',
+        { excludeImplicit: false },
+        new Set()
+      );
 
-  expect(result).toEqual([
-    'react-native-dialog',
-    'react-native-snackbar',
-    'react-native-image-picker',
-  ]);
+      expect(result).toEqual([
+        'react-native-dialog',
+        'react-native-snackbar',
+        'axios',
+        'react-native-image-picker',
+      ]);
+    });
+  });
 });
diff --git a/packages/expo/src/utils/find-all-npm-dependencies.ts b/packages/expo/src/utils/find-all-npm-dependencies.ts
@@ -1,35 +1,59 @@
-import { ProjectGraph } from '@nx/devkit';
+import { type ProjectGraph, type ProjectGraphDependency } from '@nx/devkit';
+
+// Don't want to include '@nx/react-native' and '@nx/expo' because React Native
+// autolink will warn that the package has no podspec file for iOS.
+const EXCLUDED_EXTERNAL_NODES = new Set([
+  'npm:@nx/react-native',
+  'npm:@nrwl/react-native',
+  'npm:@nx/expo',
+  'npm:@nrwl/expo',
+]);
+
+type Options = {
+  excludeImplicit: boolean;
+};
 
 export function findAllNpmDependencies(
   graph: ProjectGraph,
   projectName: string,
-  list: string[] = [],
-  seen = new Set<string>()
-) {
-  // In case of bad circular dependencies
-  if (seen.has(projectName)) {
-    return list;
-  }
+  options: Options = { excludeImplicit: false },
+  seen: Set<string> = new Set<string>()
+): string[] {
+  // Guard Case: In case of bad circular dependencies
+  if (seen.has(projectName)) return [];
   seen.add(projectName);
 
-  const node = graph.externalNodes[projectName];
-
-  // Don't want to include '@nx/react-native' and '@nx/expo' because React Native
-  // autolink will warn that the package has no podspec file for iOS.
-  if (node) {
-    if (
-      node.name !== `npm:@nx/react-native` &&
-      node.name !== `npm:@nrwl/react-native` &&
-      node.name !== `npm:@nx/expo` &&
-      node.name !== `npm:@nrwl/expo`
-    ) {
-      list.push(node.data.packageName);
-    }
-  } else {
-    // it's workspace project, search for it's dependencies
-    graph.dependencies[projectName]?.forEach((dep) =>
-      findAllNpmDependencies(graph, dep.target, list, seen)
-    );
+  // Base/Termination Case: when it finds a valid package in externalNodes
+  const node = graph.externalNodes?.[projectName];
+  if (node && !EXCLUDED_EXTERNAL_NODES.has(node.name)) {
+    return [node.data.packageName];
   }
-  return list;
+
+  // Recursive Case: Digging into related projects' dependencies
+  return (
+    (graph.dependencies[projectName] || [])
+      // Conditional filtering based on options
+      .filter(getFilterPredicate(options))
+      // this is where the recursion happens
+      .flatMap((dep) =>
+        findAllNpmDependencies(graph, dep.target, options, seen)
+      )
+  );
+}
+
+// This function is used to filter out dependencies based on the options
+// provided.
+function getFilterPredicate(options?: Options) {
+  return (dep: ProjectGraphDependency) =>
+    [
+      // base predicate returns true so it filters out nothing
+      (_pDep: ProjectGraphDependency) => true,
+
+      // conditionally filter implicit dependencies based on the option
+      ...(options?.excludeImplicit
+        ? [(pDep: ProjectGraphDependency) => pDep.type !== 'implicit']
+        : []),
+
+      // Future conditions can be added here in a similar way
+    ].every((predicate) => predicate(dep));
 }