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.  #### 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!
This commit is contained in:
parent
5a06daac7a
commit
260562e484
@ -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": []
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -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 || {})
|
||||
: [];
|
||||
|
||||
@ -1,13 +1,12 @@
|
||||
import { findAllNpmDependencies } from './find-all-npm-dependencies';
|
||||
import { DependencyType, ProjectGraph } from '@nx/devkit';
|
||||
|
||||
test('findAllNpmDependencies', () => {
|
||||
const graph: ProjectGraph = {
|
||||
const graphFixture: ProjectGraph = {
|
||||
nodes: {
|
||||
myapp: {
|
||||
type: 'app',
|
||||
name: 'myapp',
|
||||
data: { files: [] },
|
||||
data: { files: [], implicitDependencies: ['lib4'] },
|
||||
},
|
||||
lib1: {
|
||||
type: 'lib',
|
||||
@ -24,6 +23,11 @@ test('findAllNpmDependencies', () => {
|
||||
name: 'lib3',
|
||||
data: { files: [] },
|
||||
},
|
||||
lib4: {
|
||||
type: 'lib',
|
||||
name: 'lib4',
|
||||
data: { files: [] },
|
||||
},
|
||||
} as any,
|
||||
externalNodes: {
|
||||
'npm:react-native-image-picker': {
|
||||
@ -58,11 +62,20 @@ test('findAllNpmDependencies', () => {
|
||||
packageName: '@nx/react-native',
|
||||
},
|
||||
},
|
||||
'npm:axios': {
|
||||
type: 'npm',
|
||||
name: 'npm:axios',
|
||||
data: {
|
||||
version: '1',
|
||||
packageName: 'axios',
|
||||
},
|
||||
},
|
||||
},
|
||||
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',
|
||||
@ -90,10 +103,36 @@ test('findAllNpmDependencies', () => {
|
||||
target: 'npm:react-native-dialog',
|
||||
},
|
||||
],
|
||||
lib4: [
|
||||
{
|
||||
type: DependencyType.static,
|
||||
source: 'lib4',
|
||||
target: 'npm:axios',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
const result = findAllNpmDependencies(graph, 'myapp');
|
||||
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',
|
||||
@ -101,3 +140,21 @@ test('findAllNpmDependencies', () => {
|
||||
'react-native-image-picker',
|
||||
]);
|
||||
});
|
||||
|
||||
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',
|
||||
'axios',
|
||||
'react-native-image-picker',
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -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);
|
||||
// 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];
|
||||
}
|
||||
} else {
|
||||
// it's workspace project, search for it's dependencies
|
||||
graph.dependencies[projectName]?.forEach((dep) =>
|
||||
findAllNpmDependencies(graph, dep.target, list, seen)
|
||||
|
||||
// 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)
|
||||
)
|
||||
);
|
||||
}
|
||||
return list;
|
||||
|
||||
// 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));
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user