cleanup(core): move package json script handling to package json plugin (#18659)

This commit is contained in:
Craigory Coppola 2023-08-17 13:56:48 -04:00 committed by GitHub
parent 2e1bccd0c3
commit 2d08229000
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 442 additions and 262 deletions

View File

@ -1,7 +1,7 @@
import * as memfs from 'memfs';
import '../src/utils/testing/mock-fs';
import { getNxPackageJsonWorkspacesPlugin } from './package-json-workspaces';
import { createNodeFromPackageJson } from './package-json-workspaces';
describe('nx package.json workspaces plugin', () => {
it('should build projects from package.json files', () => {
@ -15,21 +15,29 @@ describe('nx package.json workspaces plugin', () => {
name: 'lib-a',
scripts: { test: 'jest' },
}),
'packages/lib-b/package.json': JSON.stringify({
name: 'lib-b',
scripts: {
build: 'tsc',
test: 'jest',
nonNxOperation: 'rm -rf .',
},
nx: {
implicitDependencies: ['lib-a'],
includedScripts: ['build', 'test'],
targets: {
build: {
outputs: ['{projectRoot}/dist'],
},
},
},
}),
},
'/root'
);
const plugin = getNxPackageJsonWorkspacesPlugin('/root');
// Targets from package.json files are handled outside of `createNodes`,
// because they are recognized even if the package.json file is not included
// in the package manager workspaces configuration.
//
// If any project has a package.json file in its root directory, those scripts
// are targets regardless of this plugin. As such, all we have to do here is identify
// that the package.json represents an Nx project, and `normalizeProjectNodes`
// will handle the rest.
expect(plugin.createNodes[1]('package.json', null)).toMatchInlineSnapshot(`
expect(createNodeFromPackageJson('package.json', '/root'))
.toMatchInlineSnapshot(`
{
"projects": {
"root": {
@ -37,11 +45,19 @@ describe('nx package.json workspaces plugin', () => {
"projectType": "library",
"root": ".",
"sourceRoot": ".",
"targets": {
"echo": {
"executor": "nx:run-script",
"options": {
"script": "echo",
},
},
},
},
},
}
`);
expect(plugin.createNodes[1]('packages/lib-a/package.json', null))
expect(createNodeFromPackageJson('packages/lib-a/package.json', '/root'))
.toMatchInlineSnapshot(`
{
"projects": {
@ -50,6 +66,51 @@ describe('nx package.json workspaces plugin', () => {
"projectType": "library",
"root": "packages/lib-a",
"sourceRoot": "packages/lib-a",
"targets": {
"test": {
"executor": "nx:run-script",
"options": {
"script": "test",
},
},
},
},
},
}
`);
expect(createNodeFromPackageJson('packages/lib-b/package.json', '/root'))
.toMatchInlineSnapshot(`
{
"projects": {
"lib-b": {
"implicitDependencies": [
"lib-a",
],
"includedScripts": [
"build",
"test",
],
"name": "lib-b",
"projectType": "library",
"root": "packages/lib-b",
"sourceRoot": "packages/lib-b",
"targets": {
"build": {
"executor": "nx:run-script",
"options": {
"script": "build",
},
"outputs": [
"{projectRoot}/dist",
],
},
"test": {
"executor": "nx:run-script",
"options": {
"script": "test",
},
},
},
},
},
}

View File

@ -9,7 +9,10 @@ import { combineGlobPatterns } from '../src/utils/globs';
import { NX_PREFIX } from '../src/utils/logger';
import { NxPluginV2 } from '../src/utils/nx-plugin';
import { output } from '../src/utils/output';
import { PackageJson } from '../src/utils/package-json';
import {
PackageJson,
readTargetsFromPackageJson,
} from '../src/utils/package-json';
import { joinPathFragments } from '../src/utils/path';
export function getNxPackageJsonWorkspacesPlugin(root: string): NxPluginV2 {
@ -20,8 +23,13 @@ export function getNxPackageJsonWorkspacesPlugin(root: string): NxPluginV2 {
combineGlobPatterns(
getGlobPatternsFromPackageManagerWorkspaces(root, readJson)
),
(pkgJsonPath) => {
const json: PackageJson = readJson(pkgJsonPath);
(p) => createNodeFromPackageJson(p, root),
],
};
}
export function createNodeFromPackageJson(pkgJsonPath: string, root: string) {
const json: PackageJson = readJsonFile(join(root, pkgJsonPath));
return {
projects: {
[json.name]: buildProjectConfigurationFromPackageJson(
@ -31,13 +39,10 @@ export function getNxPackageJsonWorkspacesPlugin(root: string): NxPluginV2 {
),
},
};
},
],
};
}
export function buildProjectConfigurationFromPackageJson(
packageJson: { name: string },
packageJson: PackageJson,
path: string,
nxJson: NxJsonConfiguration
): ProjectConfiguration & { name: string } {
@ -69,6 +74,8 @@ export function buildProjectConfigurationFromPackageJson(
sourceRoot: directory,
name,
projectType,
...packageJson.nx,
targets: readTargetsFromPackageJson(packageJson),
};
}

View File

@ -1 +1,13 @@
export const projectFilePatterns = ['package.json'];
import type { NxPluginV2 } from '../src/utils/nx-plugin';
import { workspaceRoot } from '../src/utils/workspace-root';
import { createNodeFromPackageJson } from './package-json-workspaces';
const plugin: NxPluginV2 = {
name: 'nx-all-package-jsons-plugin',
createNodes: [
'*/**/package.json',
(f) => createNodeFromPackageJson(f, workspaceRoot),
],
};
module.exports = plugin;

View File

@ -1,7 +1,13 @@
import * as memfs from 'memfs';
import '../src/utils/testing/mock-fs';
import { getNxProjectJsonPlugin } from './project-json';
import { PackageJson } from '../src/utils/package-json';
import {
getNxProjectJsonPlugin,
mergeNpmScriptsWithTargets,
} from './project-json';
describe('nx project.json plugin', () => {
it('should build projects from project.json', () => {
@ -14,9 +20,18 @@ describe('nx project.json plugin', () => {
'packages/lib-a/project.json': JSON.stringify({
name: 'lib-a',
targets: {
build: {
executor: 'nx:run-commands',
options: {},
},
},
}),
'packages/lib-a/package.json': JSON.stringify({
name: 'lib-a',
scripts: {
build: 'should not see me',
test: 'jest',
},
}),
},
'/root'
@ -44,12 +59,172 @@ describe('nx project.json plugin', () => {
"name": "lib-a",
"root": "packages/lib-a",
"targets": {
"build": {
"executor": "nx:run-commands",
"options": {},
},
"test": {
"executor": "nx:run-script",
"options": {
"script": "test",
},
},
},
},
},
}
`);
});
describe('mergeNpmScriptsWithTargets', () => {
const packageJson: PackageJson = {
name: 'my-app',
version: '0.0.0',
scripts: {
build: 'echo 1',
},
};
const packageJsonBuildTarget = {
executor: 'nx:run-script',
options: {
script: 'build',
},
};
it('should prefer project.json targets', () => {
const projectJsonTargets = {
build: {
executor: 'nx:run-commands',
options: {
command: 'echo 2',
},
},
};
const result = mergeNpmScriptsWithTargets(
packageJson,
projectJsonTargets
);
expect(result).toEqual(projectJsonTargets);
});
it('should provide targets from project.json and package.json', () => {
const projectJsonTargets = {
clean: {
executor: 'nx:run-commands',
options: {
command: 'echo 2',
},
},
};
const result = mergeNpmScriptsWithTargets(
packageJson,
projectJsonTargets
);
expect(result).toEqual({
...projectJsonTargets,
build: packageJsonBuildTarget,
});
});
it('should contain extended options from nx property in package.json', () => {
const result = mergeNpmScriptsWithTargets(
{
name: 'my-other-app',
version: '',
scripts: {
build: 'echo 1',
},
nx: {
targets: {
build: {
outputs: ['custom'],
},
},
},
},
null
);
expect(result).toEqual({
build: { ...packageJsonBuildTarget, outputs: ['custom'] },
});
});
it('should work when project.json targets is null', () => {
const result = mergeNpmScriptsWithTargets(packageJson, null);
expect(result).toEqual({
build: {
executor: 'nx:run-script',
options: {
script: 'build',
},
},
});
});
it("should work when project root is ''", () => {
const result = mergeNpmScriptsWithTargets(
{
name: 'my-app',
version: '',
scripts: {
test: 'echo testing',
},
},
{
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
}
);
expect(result).toEqual({
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
test: {
executor: 'nx:run-script',
options: { script: 'test' },
},
});
});
it('should ignore scripts that are not in includedScripts', () => {
const result = mergeNpmScriptsWithTargets(
{
name: 'included-scripts-test',
version: '',
scripts: {
test: 'echo testing',
fail: 'exit 1',
},
nx: {
includedScripts: ['test'],
},
},
{
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
}
);
expect(result).toEqual({
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
test: {
executor: 'nx:run-script',
options: { script: 'test' },
},
});
});
});
});

View File

@ -1,9 +1,17 @@
import { dirname, join } from 'node:path';
import { existsSync } from 'node:fs';
import { ProjectConfiguration } from '../src/config/workspace-json-project-json';
import {
ProjectConfiguration,
TargetConfiguration,
} from '../src/config/workspace-json-project-json';
import { toProjectName } from '../src/config/workspaces';
import { readJsonFile } from '../src/utils/fileutils';
import { NxPluginV2 } from '../src/utils/nx-plugin';
import {
PackageJson,
readTargetsFromPackageJson,
} from '../src/utils/package-json';
export function getNxProjectJsonPlugin(root: string): NxPluginV2 {
return {
@ -13,6 +21,7 @@ export function getNxProjectJsonPlugin(root: string): NxPluginV2 {
(file) => {
const json = readJsonFile<ProjectConfiguration>(join(root, file));
const project = buildProjectFromProjectJson(json, file);
mergePackageJsonConfigurationWithProjectJson(project, root);
return {
projects: {
[project.name]: project,
@ -33,3 +42,46 @@ export function buildProjectFromProjectJson(
...json,
};
}
export function mergePackageJsonConfigurationWithProjectJson(
p: ProjectConfiguration,
root: string
) {
if (existsSync(join(root, p.root, 'package.json'))) {
try {
const packageJson: PackageJson = readJsonFile(
join(root, p.root, 'package.json')
);
p.targets = mergeNpmScriptsWithTargets(packageJson, p.targets);
const { nx } = packageJson;
if (nx?.tags) {
p.tags = [...(p.tags || []), ...nx.tags];
}
if (nx?.implicitDependencies) {
p.implicitDependencies = [
...(p.implicitDependencies || []),
...nx.implicitDependencies,
];
}
if (nx?.namedInputs) {
p.namedInputs = { ...(p.namedInputs || {}), ...nx.namedInputs };
}
} catch (e) {
// ignore json parser errors
}
}
}
export function mergeNpmScriptsWithTargets(
packageJson: PackageJson,
targets: Record<string, TargetConfiguration>
): Record<string, TargetConfiguration> {
try {
return { ...readTargetsFromPackageJson(packageJson), ...(targets || {}) };
} catch (e) {
return targets;
}
}

View File

@ -9,6 +9,7 @@ const libConfig = (root, name?: string) => ({
projectType: 'library',
root: `libs/${root}`,
sourceRoot: `libs/${root}/src`,
targets: {},
});
const packageLibConfig = (root, name?: string) => ({
@ -16,6 +17,7 @@ const packageLibConfig = (root, name?: string) => ({
root,
sourceRoot: root,
projectType: 'library',
targets: {},
});
describe('Workspaces', () => {
@ -84,6 +86,7 @@ describe('Workspaces', () => {
root: 'libs/lib1',
sourceRoot: 'libs/lib1/src',
projectType: 'library',
targets: {},
});
expect(projects.lib2).toEqual(lib2Config);
expect(projects['domain-lib3']).toEqual(domainPackageConfig);
@ -124,6 +127,7 @@ describe('Workspaces', () => {
root: 'packages/my-package',
sourceRoot: 'packages/my-package',
projectType: 'library',
targets: {},
});
}
);

View File

@ -186,8 +186,6 @@ export function getRelativeProjectJsonSchemaPath(
function readAndCombineAllProjectConfigurations(tree: Tree): {
[name: string]: ProjectConfiguration;
} {
const nxJson = readNxJson(tree);
/**
* We can't update projects that come from plugins anyways, so we are going
* to ignore them for now. Plugins should add their own add/create/update methods

View File

@ -1,14 +1,10 @@
import { join } from 'path';
import { existsSync } from 'fs';
import { workspaceRoot } from '../../utils/workspace-root';
import {
ProjectGraphProcessorContext,
ProjectGraphProjectNode,
} from '../../config/project-graph';
import { mergeNpmScriptsWithTargets } from '../../utils/project-graph-utils';
import { ProjectGraphBuilder } from '../project-graph-builder';
import { PackageJson } from '../../utils/package-json';
import { readJsonFile } from '../../utils/fileutils';
import { NxJsonConfiguration } from '../../config/nx-json';
import {
ProjectConfiguration,
@ -45,36 +41,6 @@ export async function normalizeProjectNodes(
for (const key of projects) {
const p = ctx.projectsConfigurations.projects[key];
const projectRoot = join(workspaceRoot, p.root);
// Todo(@AgentEnder) we can move a lot of this to
// builtin plugin inside workspaces.ts, but there would be some functional differences
// - The plugin would only apply to package.json files found via the workspaces globs
// - This means that scripts / tags / etc from the `nx` property wouldn't be read if a project
// is being found by project.json and not included in the workspaces configuration. Maybe this is fine?
if (existsSync(join(projectRoot, 'package.json'))) {
p.targets = mergeNpmScriptsWithTargets(projectRoot, p.targets);
try {
const { nx }: PackageJson = readJsonFile(
join(projectRoot, 'package.json')
);
if (nx?.tags) {
p.tags = [...(p.tags || []), ...nx.tags];
}
if (nx?.implicitDependencies) {
p.implicitDependencies = [
...(p.implicitDependencies || []),
...nx.implicitDependencies,
];
}
if (nx?.namedInputs) {
p.namedInputs = { ...(p.namedInputs || {}), ...nx.namedInputs };
}
} catch {
// ignore json parser errors
}
}
p.implicitDependencies = normalizeImplicitDependencies(
key,

View File

@ -6,6 +6,7 @@ import {
PackageJson,
PackageJsonTargetConfiguration,
readModulePackageJson,
readTargetsFromPackageJson,
} from './package-json';
describe('buildTargetFromScript', () => {
@ -44,6 +45,76 @@ describe('buildTargetFromScript', () => {
});
});
describe('readTargetsFromPackageJson', () => {
const packageJson: PackageJson = {
name: 'my-app',
version: '0.0.0',
scripts: {
build: 'echo 1',
},
};
const packageJsonBuildTarget = {
executor: 'nx:run-script',
options: {
script: 'build',
},
};
it('should read targets from project.json and package.json', () => {
const result = readTargetsFromPackageJson(packageJson);
expect(result).toEqual({
build: {
executor: 'nx:run-script',
options: {
script: 'build',
},
},
});
});
it('should contain extended options from nx property in package.json', () => {
const result = readTargetsFromPackageJson({
name: 'my-other-app',
version: '',
scripts: {
build: 'echo 1',
},
nx: {
targets: {
build: {
outputs: ['custom'],
},
},
},
});
expect(result).toEqual({
build: { ...packageJsonBuildTarget, outputs: ['custom'] },
});
});
it('should ignore scripts that are not in includedScripts', () => {
const result = readTargetsFromPackageJson({
name: 'included-scripts-test',
version: '',
scripts: {
test: 'echo testing',
fail: 'exit 1',
},
nx: {
includedScripts: ['test'],
},
});
expect(result).toEqual({
test: {
executor: 'nx:run-script',
options: { script: 'test' },
},
});
});
});
const rootPackageJson: PackageJson = readJsonFile(
join(workspaceRoot, 'package.json')
);

View File

@ -134,6 +134,16 @@ export function buildTargetFromScript(
};
}
export function readTargetsFromPackageJson({ scripts, nx }: PackageJson) {
const res: Record<string, TargetConfiguration> = {};
Object.keys(scripts || {}).forEach((script) => {
if (!nx?.includedScripts || nx?.includedScripts.includes(script)) {
res[script] = buildTargetFromScript(script, nx);
}
});
return res;
}
/**
* Uses `require.resolve` to read the package.json for a module.
*

View File

@ -1,21 +1,5 @@
let jsonFileOverrides: Record<string, any> = {};
jest.mock('nx/src/utils/fileutils', () => ({
...(jest.requireActual('nx/src/utils/fileutils') as any),
readJsonFile: (path) => {
if (path.endsWith('nx.json')) return {};
if (!(path in jsonFileOverrides))
throw new Error('Tried to read non-mocked json file: ' + path);
return jsonFileOverrides[path];
},
}));
import { PackageJson } from './package-json';
import { ProjectGraph } from '../config/project-graph';
import {
getSourceDirOfDependentProjects,
mergeNpmScriptsWithTargets,
} from './project-graph-utils';
import { getSourceDirOfDependentProjects } from './project-graph-utils';
describe('project graph utils', () => {
describe('getSourceDirOfDependentProjects', () => {
@ -119,158 +103,4 @@ describe('project graph utils', () => {
});
});
});
describe('mergeNpmScriptsWithTargets', () => {
const packageJson: PackageJson = {
name: 'my-app',
version: '0.0.0',
scripts: {
build: 'echo 1',
},
};
const packageJsonBuildTarget = {
executor: 'nx:run-script',
options: {
script: 'build',
},
};
beforeAll(() => {
jsonFileOverrides['apps/my-app/package.json'] = packageJson;
});
afterAll(() => {
jsonFileOverrides = {};
});
it('should prefer project.json targets', () => {
const projectJsonTargets = {
build: {
executor: 'nx:run-commands',
options: {
command: 'echo 2',
},
},
};
const result = mergeNpmScriptsWithTargets(
'apps/my-app',
projectJsonTargets
);
expect(result).toEqual(projectJsonTargets);
});
it('should provide targets from project.json and package.json', () => {
const projectJsonTargets = {
clean: {
executor: 'nx:run-commands',
options: {
command: 'echo 2',
},
},
};
const result = mergeNpmScriptsWithTargets(
'apps/my-app',
projectJsonTargets
);
expect(result).toEqual({
...projectJsonTargets,
build: packageJsonBuildTarget,
});
});
it('should contain extended options from nx property in package.json', () => {
jsonFileOverrides['apps/my-other-app/package.json'] = {
name: 'my-other-app',
scripts: {
build: 'echo 1',
},
nx: {
targets: {
build: {
outputs: ['custom'],
},
},
},
};
const result = mergeNpmScriptsWithTargets('apps/my-other-app', null);
expect(result).toEqual({
build: { ...packageJsonBuildTarget, outputs: ['custom'] },
});
});
it('should work when project.json targets is null', () => {
const result = mergeNpmScriptsWithTargets('apps/my-app', null);
expect(result).toEqual({
build: {
executor: 'nx:run-script',
options: {
script: 'build',
},
},
});
});
it("should work when project root is ''", () => {
jsonFileOverrides['package.json'] = {
name: 'my-app',
scripts: {
test: 'echo testing',
},
};
const result = mergeNpmScriptsWithTargets('', {
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
});
expect(result).toEqual({
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
test: {
executor: 'nx:run-script',
options: { script: 'test' },
},
});
});
it('should ignore scripts that are not in includedScripts', () => {
jsonFileOverrides['includedScriptsTest/package.json'] = {
name: 'included-scripts-test',
scripts: {
test: 'echo testing',
fail: 'exit 1',
},
nx: {
includedScripts: ['test'],
},
};
const result = mergeNpmScriptsWithTargets('includedScriptsTest', {
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
});
expect(result).toEqual({
build: {
executor: 'nx:run-commands',
options: { command: 'echo hi' },
},
test: {
executor: 'nx:run-script',
options: { script: 'test' },
},
});
});
});
});

View File

@ -28,27 +28,6 @@ export function projectHasTargetAndConfiguration(
);
}
export function mergeNpmScriptsWithTargets(
projectRoot: string,
targets
): Record<string, TargetConfiguration> {
try {
const { scripts, nx }: PackageJson = readJsonFile(
join(projectRoot, 'package.json')
);
const res: Record<string, TargetConfiguration> = {};
// handle no scripts
Object.keys(scripts || {}).forEach((script) => {
if (!nx?.includedScripts || nx?.includedScripts.includes(script)) {
res[script] = buildTargetFromScript(script, nx);
}
});
return { ...res, ...(targets || {}) };
} catch (e) {
return targets;
}
}
export function getSourceDirOfDependentProjects(
projectName: string,
projectGraph = readCachedProjectGraph()

View File

@ -12,3 +12,18 @@ jest.mock('fs', (): Partial<typeof import('fs')> => {
},
};
});
// @ts-ignore
jest.mock('node:fs', (): Partial<typeof import('fs')> => {
const mockFs = require('memfs').fs;
return {
...mockFs,
existsSync(path: string) {
if (path.endsWith('.node')) {
return true;
} else {
return mockFs.existsSync(path);
}
},
};
});